1171016 - ParseFTPList.cpp: Value stored to 'linelen' is never read

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

Found by scan-build:
https://people.mozilla.org/~sledru/reports/fx-scan-build/report-ParseFTPList.cpp-ParseFTPList-15-1.html#EndPath

https://dxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/ParseFTPList.cpp?from=ParseFTPList.cpp#40

Comment 1

[Piervincenzo Parisi [:PierV]]

I'd like to work in this.

It looks like it has been put there as a variable initialization, which happens anyway at https://dxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/ParseFTPList.cpp?from=ParseFTPList.cpp#50

Should we romove it?

Comment 2

[Sylvestre Ledru [:Sylvestre]]

yes (as the title of this bug suggests ;)

Comment 3

[Sanchit Kumar (beginner)]

If no one is working on it. Should i go ahead and submit a patch? :)

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Piervincenzo seems to work on this. If he/she didn't proposed a patch in a few days, you are welcome to take it.

Comment 5

[Sanchit Kumar (beginner)]

Okay. :)

Comment 6

[Sanchit Kumar (beginner)]

Attachment: bug1171016_initializationAtDeclaration.diff

Initialized the variable 'linelen' at its declaration.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8617868 [details] [diff] [review]
bug1171016_initializationAtDeclaration.diff

lgtm but I am not the owner of this code.

Comment 8

[Sanchit Kumar (beginner)]

Okay.

Comment 9

[Patrick McManus [:mcmanus]]

Comment on attachment 8617868 [details] [diff] [review]
bug1171016_initializationAtDeclaration.diff

Review of attachment 8617868 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!

::: netwerk/streamconv/converters/ParseFTPList.cpp
@@ -23,5 @@
>  int ParseFTPList(const char *line, struct list_state *state,
>                   struct list_result *result )
>  {
>    unsigned int carry_buf_len; /* copy of state->carry_buf_len */
> -  unsigned int linelen, pos;

I would rather fix this by declaring the variable where it is first meaningfully used.

remove lineline from this declaration

@@ -36,5 @@
>    /* carry buffer is only valid from one line to the next */
>    carry_buf_len = state->carry_buf_len;
>    state->carry_buf_len = 0;
>  
> -  linelen = 0;

remove this too (as you did)

and then down below where we have "linelen = p - line;" just change that to also be the declaration: "unsigned int linelen = p - line;"

Comment 10

[Sanchit Kumar (beginner)]

Okay. I will fix that now.
Thanks.

Comment 11

[Sanchit Kumar (beginner)]

Attachment: bug1171016_initializationAtDeclaration.diff

Changes made to declare the variable 'linelen' where it is being used.

Comment 12

[Patrick McManus [:mcmanus]]

thanks sanchit

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/821d5d971f7c

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/821d5d971f7c