Eric Blake <ebl...@redhat.com> writes:
> On 02/06/2014 07:30 AM, Markus Armbruster wrote:
>> Visitors get passed a pointer to the visited object. The generated
>> visitors try to cope with this pointer being null in some places, for
>> instance like this:
>>
>> visit_start_optional(m, obj ? &(*obj)->has_name : NULL, "name", &err);
>>
>> visit_start_optional() passes its second argument to Visitor method
>> start_optional. Two out of two methods dereference it
>> unconditionally.
>
> Let's see if I can find them without Coverity's help...
>
> opts-visitor.c:opts_start_optional
> qmp-input-visitor.c:qmp_input_start_optional
> string-input-visitor.c:parse_start_optional
>
> Your counting is off :) All three unconditionally dereference.
Right. I can't count :)
Most Coverity's defects are about silliness like this:
visit_start_optional(m, obj ? &(*obj)->has_name : NULL, "name", &err);
if (obj && (*obj)->has_name) {
visit_type_str(m, obj ? &(*obj)->name : NULL, "name", &err);
}
The second "obj ? ... : ..." is guarded by if (obj), thus cannot take
the false branch.
> [While at it, this code style from parse_start_optional, and similar in
> other locations, is rather verbose:
>
> if (!siv->string) {
> *present = false;
> return;
> }
>
> *present = true;
> }
>
> Shorter would be:
>
> *present = !!siv->string;
> }
>
> but that doesn't affect this patch]
Coccinelle job, if I can teach it our macros. Without that, it
frequently chokes on some macro usage, leaving too much code unexamined.
>> I fail to see how hits pointer could legitimately be null.
>>
>> All this useless null checking is highly redundant, which Coverity
>> duly reports. About 200 times.
>>
>> Remove the useless null checks.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> scripts/qapi-visit.py | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
