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 <jamestio...@gmail.com>
> Reviewed-by: Simon Horman <simon.hor...@corigine.com>
> ---
> 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?
> return hmap_count(&shash->map);
> }
>
> diff --git a/lib/simap.c b/lib/simap.c
> index 0ee08d74d..1c01d4ebe 100644
> --- a/lib/simap.c
> +++ b/lib/simap.c
> @@ -17,6 +17,7 @@
> #include <config.h>
> #include "simap.h"
> #include "hash.h"
> +#include "util.h"
>
> static size_t hash_name(const char *, size_t length);
> static struct simap_node *simap_find__(const struct simap *,
> @@ -84,6 +85,7 @@ simap_is_empty(const struct simap *simap)
> size_t
> simap_count(const struct simap *simap)
> {
> + ovs_assert(simap);
> return hmap_count(&simap->map);
> }
>
> diff --git a/lib/smap.c b/lib/smap.c
> index 47fb34502..122adca27 100644
> --- a/lib/smap.c
> +++ b/lib/smap.c
> @@ -300,6 +300,7 @@ smap_is_empty(const struct smap *smap)
> size_t
> smap_count(const struct smap *smap)
> {
> + ovs_assert(smap);
> return hmap_count(&smap->map);
> }
>
> --
> 2.40.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
