On 12 Jul 2023, at 0:34, Ilya Maximets wrote:
> On 7/11/23 12:05, Eelco Chaudron wrote:
>>
>>
>> On 13 Jun 2023, at 20:34, James Raphael Tiovalen wrote:
>>
>>> This commit adds assertions in the functions `shash_count`,
>>> `simap_count`, and `smap_count` to ensure that the corresponding input
>>> struct pointer is not NULL.
>>>
>>> This ensures that if the return values of `shash_sort`, `simap_sort`,
>>> or `smap_sort` are NULL, then the following for loops would not attempt
>>> to access the pointer, which might result in segmentation faults or
>>> undefined behavior.
>>>
>>> Signed-off-by: James Raphael Tiovalen <[email protected]>
>>> Reviewed-by: Simon Horman <[email protected]>
>>> ---
>>> lib/shash.c | 2 ++
>>> lib/simap.c | 2 ++
>>> lib/smap.c | 1 +
>>> 3 files changed, 5 insertions(+)
>>>
>>> diff --git a/lib/shash.c b/lib/shash.c
>>> index a7b2c6458..2bfc8eb50 100644
>>> --- a/lib/shash.c
>>> +++ b/lib/shash.c
>>> @@ -17,6 +17,7 @@
>>> #include <config.h>
>>> #include "openvswitch/shash.h"
>>> #include "hash.h"
>>> +#include "util.h"
>>>
>>> static struct shash_node *shash_find__(const struct shash *,
>>> const char *name, size_t name_len,
>>> @@ -100,6 +101,7 @@ shash_is_empty(const struct shash *shash)
>>> size_t
>>> shash_count(const struct shash *shash)
>>> {
>>> + ovs_assert(shash);
>>
>> My preference would be to return 0, in these instances. What do others think?
>
> Calling these function with a NULL argument doesn't make much sense to me.
> free()-like functions should generally accept NULL pointers, but functions
> that actually do work on a datastructure shouldn't, IMO.
Fine by me too, so with this:
Acked-by: Eelco Chaudron <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
