Re: [Patch size_t V3 16/19] Convert various things to size_t
From: Junio C Hamano <hidden>
Date: 2017-08-21 06:35:06
Martin Koegler [off-list ref] writes:
quoted hunk
From: Martin Koegler <redacted> Signed-off-by: Martin Koegler <redacted> --- 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.
quoted hunk
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.
quoted hunk
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.
quoted hunk
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.
quoted hunk
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.
quoted hunk
@@ -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.