Re: [Patch size_t V3 16/19] Convert various things to size_t
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 21, 2017, 06:34 UTC
- Message-ID
- <xmqqziat5sz9.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1502914591-26215-17-git-send-email-martin@mail.zuhause>
Martin Koegler <martin.koegler@chello.at> writes:
Show 23 quoted lines
> From: Martin Koegler <martin.koegler@chello.at> > > Signed-off-by: Martin Koegler <martin.koegler@chello.at> > --- > bisect.c | 2 +- > blame.c | 2 +- > builtin/fmt-merge-msg.c | 2 +- > builtin/mktag.c | 2 +- > dir.c | 4 ++-- > dir.h | 2 +- > 6 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/bisect.c b/bisect.c > index 2549eaf..0580c82 100644 > --- a/bisect.c > +++ b/bisect.c > @@ -131,7 +131,7 @@ static void show_list(const char *debug, int counted, int nr, > struct commit *commit = p->item; > unsigned flags = commit->object.flags; > enum object_type type; > - unsigned long size; > + size_t size; > char *buf = read_sha1_file(commit->object.sha1, &type, &size);
Hmph. At this point in the series, read_sha1_file() has already been modified to take size_t, but before this patch this caller used to pass a pointer to ulong, so between these two places in the series the code wouldn't have compiled.
But the end result is obviously consistent.
Show 13 quoted lines
> diff --git a/blame.c b/blame.c
> index 739a280..f628b42 100644
> --- a/blame.c
> +++ b/blame.c
> @@ -1621,7 +1621,7 @@ static const char *get_next_line(const char *start, const char *end)
> static int prepare_lines(struct blame_scoreboard *sb)
> {
> const char *buf = sb->final_buf;
> - unsigned long len = sb->final_buf_size;
> + size_t len = sb->final_buf_size;
> const char *end = buf + len;
> const char *p;
> int *lineno;The final_buf_size member has already been made to size_t, and this code used to receive it in ulong, which is OK for size_t <= ulong platforms (which is about everybody). OK.
Show 13 quoted lines
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index 61ab796..3faf100 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -464,7 +464,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
> static void fmt_tag_signature(struct strbuf *tagbuf,
> struct strbuf *sig,
> const char *buf,
> - unsigned long len)
> + size_t len)
> {
> const char *tag_body = strstr(buf, "\n\n");
> if (tag_body) {The callers have been passing size_t but due to the above, size_t the callers had were coerced into ulong before this function used it. Again, as long as size_t <= ulong, that was fine, and this makes things even more consistent. Good.
Show 13 quoted lines
> diff --git a/builtin/mktag.c b/builtin/mktag.c
> index 0663106..ff919a7 100644
> --- a/builtin/mktag.c
> +++ b/builtin/mktag.c
> @@ -34,7 +34,7 @@ static int verify_object(const unsigned char *sha1, const char *expected_type)
> return ret;
> }
>
> -static int verify_tag(char *buffer, unsigned long size)
> +static int verify_tag(char *buffer, size_t size)
> {
> int typelen;
> char type[20];The only caller of this function was passing .len member of a strbuf, which has been size_t forever, so this is exactly the same situation as fmt_tag_signature() above. Good.
Show 13 quoted lines
> diff --git a/dir.c b/dir.c
> index f161c26..0c7c767 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -180,7 +180,7 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
> */
> char *common_prefix(const struct pathspec *pathspec)
> {
> - unsigned long len = common_prefix_len(pathspec);
> + size_t len = common_prefix_len(pathspec);
>
> return len ? xmemdupz(pathspec->items[0].match, len) : NULL;
> }xmemdupz() takes size_t, so this could have been truncating "len" that is ulong before this change, but common_prefix_len() returns size_t anyway, so there wasn't any information loss. This change just makes the code to use the correct type that is sufficiently wide. Good.
Show 9 quoted lines
> @@ -2673,7 +2673,7 @@ static void load_sha1_stat(struct sha1_stat *sha1_stat,
> sha1_stat->valid = 1;
> }
>
> -struct untracked_cache *read_untracked_extension(const void *data, unsigned long sz)
> +struct untracked_cache *read_untracked_extension(const void *data, size_t sz)
> {
> struct untracked_cache *uc;
> struct read_data rd;Ditto. The sole caller had size_t, that used to be promoted to ulong in the old code but with thie patch, things become consistent.
Overall it looks good to me.
The "bisect.c" one feels a bit odd, though; I'd expect it to be done when read_sha1_file() is made to take a pointer to size_t to avoid intermediate breakage.
Thanks.