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? Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
