1396794 - Dead code in FlacFrameParser::DecodeHeaderBlock

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P5)

Description

[Sylvestre Ledru [:Sylvestre]]

http://sylvestre.ledru.info/reports/fx-scan-build/report-FlacFrameParser.cpp-DecodeHeaderBlock-14-1.html#EndPath
aPacket & aLength change are useless.

Comment 1

[Vedant Chakravadhanula(:vedantc98)]

I'm new to the community and I want to make my first contribution, can someone guide me regarding this?

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Sure, just submit a patch

Comment 3

[Vedant Chakravadhanula(:vedantc98)]

I've removed the two dead-code increment lines for aLength and aPacket, is that all that was needed to be done?

Comment 4

[Bharat Raghunathan]

Is this bug still available to work on()? May I submit a patch?

Comment 5

[Bharat Raghunathan]

Attachment: Bug 1396794 - Removed aPacket and aLength change

Review commit: https://reviewboard.mozilla.org/r/180288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180288/

Comment 6

[Bharat Raghunathan]

Comment on attachment 8908621 [details]
Bug 1396794 - Removed aPacket and aLength change

Can you please review my patch in Comment #5?

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Looks great but you should now find a reviewer (I am not the owner of this code)

Comment 8

[Bharat Raghunathan]

Can you please redirect me to the correct reviewer(s) of this code? Currently you have been listed as the mentor hence, I have flagged you for review. Could you please transfer the review flag/mentor status to the appropriate person(s)? Thanks.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Please see:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_-_Get_your_code_reviewed
and don't use need info to get review but the review in attachment


Here, mentor means that I am here to guide you, not to review patches :)

Comment 10

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8908621 [details]
Bug 1396794 - Removed aPacket and aLength change

https://reviewboard.mozilla.org/r/180288/#review185842

Comment 11

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Thanks for the patch. For future reference, you don't need to do a needinfo? as well as a review?

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/24833bf7fb61

Comment 13

[Bharat Raghunathan]

Is this all or is there anything more that needs to be done? Thanks <@kentuckyfriedtakahe,@sylvestre >

Comment 14

[Sylvestre Ledru [:Sylvestre]]

That is all, well done!