Daniel Shahaf wrote: > Andreas Stieger wrote on Sat, Nov 16, 2013 at 00:08:40 +0000: >> subversion/tests/libsvn_fs_base/fs-base-test.c:239:6: warning: 'present' >> may be used uninitialized in this function [-Wmaybe-uninitialized] et >> al. Initializing to boolean value that would fail the test if the >> reference wasn't changed in called function at all. >> >> [[[ >> * subversion/tests/libsvn_fs/fs-test.c >> (check_entry_present, check_entry_absent), >> subversion/tests/libsvn_fs_base/fs-base-test.c [...] >> (check_entry_present, check_entry_absent, check_id_present, >> check_id_absent): initialize variable present to silence >> warning -Wmaybe-uninitialized >> ]]]
This looks to me like a false warning. I can see by inspection that check_entry() always initializes this variable or returns an error, and in the latter case the variable isn't used. As check_entry() is a local function, and is part of the test rather than part of the system under test, it's not necessary to be so careful: we can trust it just as much as we can trust its callers. So the only reason to change anything is to silence the warning. In general I don't want to add extra code just to get rid of spurious warnings. However, in this case I have no objection to the =FALSE / =TRUE variant. >> Andreas > >> Index: subversion/tests/libsvn_fs/fs-test.c >> =================================================================== [...] >> - svn_boolean_t present; >> + svn_boolean_t present = false; >> SVN_ERR(check_entry(root, path, name, &present, pool)); >> >> if (! present) [...] > > Personally I'd consider a tristate here, to allow the initialization to > be the same regardless of the sense of the condition in the 'if'. Ugh, that's OTT. The function's result is already tri-state: it yields TRUE or FALSE or returns an error. > If you keep it a boolean, you should use FALSE and TRUE instead; those > are in C89, and 'false'/'true' aren't. > > If you make either of those two changes, +1 to commit. (If further > changes are needed, they can be made in subsequent commits.) - Julian
