[RFC PATCH 4/5] cli: change the data structure for notmuch address deduplication

[David Bremner]

Jani Nikula <jani at nikula.org> writes:

>  
> +static int
> +strcase_equal (const void *a, const void *b)
> +{
> +    return strcasecmp (a, b) == 0;
> +}
> +
> +static unsigned int
> +strcase_hash (const void *ptr)
> +{
> +    const char *s = ptr;
> +
> +    /* This is the djb2 hash. */
> +    unsigned int hash = 5381;
> +    while (s && *s) {
> +	hash = ((hash << 5) + hash) + tolower (*s);
> +	s++;
> +    }
> +
> +    return hash;
> +}
> +

as discussed, these functions probably need to be factored out into
libutil.

> +	l = g_list_find_custom (list, mailbox, mailbox_compare);
> +	if (l) {
> +	    talloc_free (mailbox);
> +	    mailbox = l->data;
> +	    mailbox->count++;
> +	    return TRUE;
> +	}

I found this use of mailbox as a temporary variable confusing; despite
the obvious return I thought it might have something to do with the
g_list_append below.  Maybe just make a block scope temporary variable?

> +
> +	g_list_append (list, mailbox);
> +	return FALSE;
>      }
>  
> -    mailbox = new_mailbox (ctx->format, name, addr);
> -    if (! mailbox)
> +    key = talloc_strdup (ctx->format, addr);
> +    if (! key)
>  	return FALSE;

I guess this doesn't make the error handling worse; both old and new
code silently ignore OOM if I understand correctly. Do you happen to
understand the original choice of using ctx->format rather than that
ctx->notmuch for a talloc parent? it doesn't seem to get deallocated any
earlier.

d