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),
>> (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.
>> 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.)
Received on 2013-11-18 14:40:38 CET