Philippe Mathieu-Daudé <phi...@redhat.com> writes: > On 7/7/20 7:48 AM, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <phi...@redhat.com> writes: >> >>> Coverity noticed commit 950c4e6c94 introduced a dereference before >>> null check in get_opt_value (CID1391003): >>> >>> In get_opt_value: All paths that lead to this null pointer >>> comparison already dereference the pointer earlier (CWE-476) >>> >>> We fixed this in commit 6e3ad3f0e31, but relaxed the check in commit >>> 0c2f6e7ee99 because "No callers of get_opt_value() pass in a NULL >>> for the 'value' parameter". >>> >>> Since this function is publicly exposed, it risks new users to do >>> the same error again. Avoid that documenting the 'value' argument >>> must not be NULL. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >>> --- >>> v2: Drop confuse comment (Damien Hedde) >>> --- >>> include/qemu/option.h | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/include/qemu/option.h b/include/qemu/option.h >>> index eb4097889d..ac50d25774 100644 >>> --- a/include/qemu/option.h >>> +++ b/include/qemu/option.h >>> @@ -28,6 +28,19 @@ >>> >>> #include "qemu/queue.h" >>> >>> +/** >>> + * get_opt_value >>> + * @p: a pointer to the option name, delimited by commas >>> + * @value: a non-NULL pointer that will received the delimited options >> >> s/received/receive/ >> >>> + * >>> + * The @value char pointer will be allocated and filled with >>> + * the delimited options. >>> + * >>> + * Returns the position of the comma delimiter/zero byte after the >>> + * option name in @p. >>> + * The memory pointer in @value must be released with a call to g_free() >>> + * when no longer required. >>> + */ >>> const char *get_opt_value(const char *p, char **value); >>> >>> void parse_option_size(const char *name, const char *value, >> >> You are adding a *second* doc comment: the definition already has one. >> It's clearer than yours on some things, and less explicit on others. >> Feel free to improve or replace it. But do put it next to the >> definition. > > Hmm I haven't noticed it, because my reflex is to look at the usage > description in the prototype declaration, not in the implementation. > > I know, 2 different schools. > > Maybe we can make both schools less unhappy by simply duplicating the > function description in both the header and the source files...
Worst of both worlds :) [...]
