Re: [PATCH v2 1/5] mailmap: add a function to inspect the number of entries
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jan 4, 2021, 15:14 UTC
- Message-ID
- <87im8cvinn.fsf@evledraar.gmail.com>
- In-Reply-To
- <20210103211849.2691287-2-sandals@crustytoothpaste.net>
On Sun, Jan 03 2021, brian m. carlson wrote:
Show 24 quoted lines
> We're soon going to change the type of our mailmap into an opaque struct
> so we can add features and improve performance. When we do so, it won't
> be possible for users to inspect its internals to determine how many
> items are present, so let's introduce a function that lets users inquire
> how many objects are in the mailmap and use it where we want this
> information.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> mailmap.c | 5 +++++
> mailmap.h | 1 +
> pretty.c | 2 +-
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/mailmap.c b/mailmap.c
> index 962fd86d6d..c9a538f4e2 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -361,3 +361,8 @@ int map_user(struct string_list *map,
> debug_mm("map_user: --\n");
> return 0;
> }
> +
> +int mailmap_entries(struct string_list *map)We're returning ->nr here from string_list, whose "nr" is unsigned int. Wouldn't it make more sense to do the same here?...
Show 25 quoted lines
> +{
> + return map->nr;
> +}
> diff --git a/mailmap.h b/mailmap.h
> index d0e65646cb..ff57b05a15 100644
> --- a/mailmap.h
> +++ b/mailmap.h
> @@ -5,6 +5,7 @@ struct string_list;
>
> int read_mailmap(struct string_list *map, char **repo_abbrev);
> void clear_mailmap(struct string_list *map);
> +int mailmap_entries(struct string_list *map);
>
> int map_user(struct string_list *map,
> const char **email, size_t *emaillen, const char **name, size_t *namelen);
> diff --git a/pretty.c b/pretty.c
> index 7a7708a0ea..43a0039870 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -681,7 +681,7 @@ static int mailmap_name(const char **email, size_t *email_len,
> mail_map = xcalloc(1, sizeof(*mail_map));
> read_mailmap(mail_map, NULL);
> }
> - return mail_map->nr && map_user(mail_map, email, email_len, name, name_len);
> + return mailmap_entries(mail_map) && map_user(mail_map, email, email_len, name, name_len);...and isn't this introducing a bug where the number of entries is beyond "signed int" but not "unsigned int" by overflowing?
Bikeshedding: For both hashmap and strmap we have a
{hashmap,strmap}_get_size() function. I think a "static inline unsigned
int mailmap_get_size()" here would be more consistent & obvious.> } > > static size_t format_person_part(struct strbuf *sb, char part,