Re: [PATCH 01/38] lib: add _notmuch_string_map_set

[Tomi Ollila <tomi.ollila@iki.fi>]

On Sat, Jan 16 2021, David Bremner wrote:


First, is this patch series safe to be applied and put into use ? :D

I've looked this through once. So much stuff hard to figure out
if there is something broken. 

Most of the comments here and other patches are about style (line
break), but some (IIRC at least 2) which changes functionality a bit...

> This will be used (and tested) by the configuration caching code to be
> added in the next commit.
> ---
>  lib/notmuch-private.h |  5 +++++
>  lib/string-map.c      | 15 +++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 57ec7f72..51016b0b 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -638,6 +638,11 @@ _notmuch_string_map_append (notmuch_string_map_t *map,
>  			    const char *key,
>  			    const char *value);
>  
> +void
> +_notmuch_string_map_set (notmuch_string_map_t *map,
> +			 const char *key,
> +			 const char *value);
> +
>  const char *
>  _notmuch_string_map_get (notmuch_string_map_t *map, const char *key);
>  
> diff --git a/lib/string-map.c b/lib/string-map.c
> index a88404c7..1f3215fb 100644
> --- a/lib/string-map.c
> +++ b/lib/string-map.c
> @@ -142,6 +142,21 @@ bsearch_first (notmuch_string_pair_t *array, size_t len, const char *key, bool e
>  	return NULL;
>  
>  }
> +void
> +_notmuch_string_map_set (notmuch_string_map_t *map, const char *key, const char *val)

The style in string-map.c (and matches the .h above) would indicate the abobe should be:

_notmuch_string_map_set (notmuch_string_map_t *map, 
                         const char *key, 
                         const char *val)

> +{
> +    notmuch_string_pair_t *pair;
> +
> +    /* this means that calling string_map_set invalidates iterators */
> +    _notmuch_string_map_sort (map);
> +    pair = bsearch_first (map->pairs, map->length, key, true);
> +    if (! pair)
> +       _notmuch_string_map_append (map, key, val);
> +    else {
> +       talloc_free (pair->value);
> +       pair->value = talloc_strdup (map->pairs, val);
> +    }
> +}
>  
>  const char *
>  _notmuch_string_map_get (notmuch_string_map_t *map, const char *key)
> -- 
> 2.29.2