[Toybox] [PATCH] fix use of uninitialized value in ls

[enh]

On Fri, Dec 11, 2015 at 11:35 PM, Rob Landley <rob at landley.net> wrote:
> On Fri, Dec 11, 2015 at 5:26 PM, enh <enh at google.com> wrote:
>> actually found by cferris@ *running* valgrind, but ykwim.
>
> --- a/toys/posix/ls.c
> +++ b/toys/posix/ls.c
> @@ -299,6 +299,7 @@ static void listfiles(int dirfd, struct dirtree *indir)
>    }
>
>    memset(totals, 0, sizeof(totals));
> +  memset(len, 0, sizeof(len));
>
> Um, doesn't entrylen() set the len fields? (It uses len as output only?)
>
> The entrylen() code is a bunch of conditional assignments, but the
> same conditionals should be on the users of those fields? Let's see,
> use of len on 352 is after entrylen() on 350, 379-384 are after
> entrylen() on 378, 413 and 421 are after 408, and final user on 498 is
> also after 408. So yes, entrylen() is always called to initialize
> len() before len is used.
>
> All those users except line 352 are just *len, I.E. len[0], which is
> unconditionally set by entrylen(). I suspect what valgrind is
> complaining about is that the conditionally set fields (ala len[1] on
> FLAG_i) are unconditionally added to the relevant totals[] column
> (makes the loop simple), but the resulting totals column is only used
> under the same flag tests that entrylen() sets. So when those flags
> aren't set, we do add uninitialized data to a column which is then
> never used. The uninitialized data is used in a write-only manner.
>
> *shrug* I can add the memset if it makes people feel better about
> valgrind, but it shouldn't change the behavior of the code in any way
> that I can see?

yeah, i haven't observed any failures from this, but not being
valgrind-clean makes it easier for serious problems to hide in the
noise.

> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.

 1449906523.0