On Fri, Dec 11, 2015 at 11:35 PM, Rob Landley <[email protected]> wrote: > On Fri, Dec 11, 2015 at 5:26 PM, enh <[email protected]> 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. _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
