[PATCH] Squash coverity warning for REVERSE_INULL in dump_note_entry_p

Luis Machado luis.machado@linaro.org
Thu Nov 19 16:42:41 GMT 2020


On 11/18/20 6:06 PM, Keith Seitz via Gdb-patches wrote:
> Coverity detected a "defect" in dump_noe_entry_p in linux-tdep.c:
> 
>    static int
>    dump_note_entry_p (filter_flags filterflags, const struct smaps_vmflags *v,
>                    int maybe_private_p, int mapping_anon_p, int mapping_file_p,
>                    const char *filename, ULONGEST addr, ULONGEST offset)
>    {
>      /* vDSO and vsyscall mappings will end up in the core file.  Don't
>         put them in the NT_FILE note.  */
>      if (strcmp ("[vdso]", filename) == 0
>          || strcmp ("[vsyscall]", filename) == 0)
>        return 0;
> 
>      /* Otherwise, any other file-based mapping should be placed in the
>         note.  */
>      return filename != nullptr;
>    }
> 
> Those strcmp's will derefernce `filename' so there is little point
> to checking whether it is non-NULL or not;  we would have already
> segfaulted.  It also cannot be nullptr because its value is read directly
> from /proc/PID/maps. The "worst" it can be is an empty string.

I don't quite get the fix. Shouldn't we check that filename is at least 
empty and not allow the comparison to go through?

Also, should we dump it if it is an empty string? I'd at least clarify 
that in the function documentation... and probably also turn the return 
type into a bool.

You didn't mention the coverity message, so I don't know what exactly it 
was complaining about. But false/true should be a better return value 
now that we support those.


More information about the Gdb-patches mailing list