Web lists-archives.com

Re: [PATCH] Convert size datatype to size_t




Hi,

On Wed, 9 Aug 2017, Junio C Hamano wrote:

> Martin Koegler <martin.koegler@xxxxxxxxx> writes:
> 
> > On Wed, Aug 09, 2017 at 09:19:06AM +0200, Martin Koegler wrote:
> >> On Tue, Aug 08, 2017 at 11:04:20PM -0700, Junio C Hamano wrote:
> >> > As https://travis-ci.org/git/git/jobs/262463159 shows, unfortunately
> >> > it turns out that things are not so simple X-<.  On Linux32, size_t
> >> > is uint, which is the same size as ulong, but "%lu" is not appropriate
> >> > for showing a size_t value.
> >> > 
> >> > So you are correct to say in the comment under three-dashes that
> >> > there is much more to change in the codebase.
> >> 
> >> I'll post a new version fixing the *printf issues.
> >
> > What is the correct format specifier for size_t?
> > Linux has %zu (C99). Is that OK for the GIT codebase?
> 
> I haven't double checked, but IIRC we cast to uintmax_t and use
> PRIuMAX.

That would be my preference, too. This is the patch that I need to make
`pu` compile for me again (note: there is one `unsigned long` -> `size_t`
change hiding between all the `%lu` -> `%PRIuMAX` changes):

-- snipsnap --
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index bc19e08a62d..10428cbeb0b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -232,7 +232,7 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,
 		if (data->mark_query)
 			data->info.sizep = &data->size;
 		else
-			strbuf_addf(sb, "%lu", data->size);
+			strbuf_addf(sb, "%" PRIuMAX, (uintmax_t)data->size);
 	} else if (is_atom("objectsize:disk", atom, len)) {
 		if (data->mark_query)
 			data->info.disk_sizep = &data->disk_size;
diff --git a/builtin/grep.c b/builtin/grep.c
index 9be8f817f2f..e3e33f3baab 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -437,7 +437,7 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
 		struct object *object;
 		struct tree_desc tree;
 		void *data;
-		unsigned long size;
+		size_t size;
 		struct strbuf base = STRBUF_INIT;
 
 		object = parse_object_or_die(oid, oid_to_hex(oid));
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 3ab99c47a0c..b09d9cba7e4 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -99,7 +99,7 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,
 						  "BAD");
 				else
 					xsnprintf(size_text, sizeof(size_text),
-						  "%lu", size);
+						  "%" PRIuMAX, (uintmax_t)size);
 			} else
 				xsnprintf(size_text, sizeof(size_text), "-");
 			printf("%06o %s %s %7s\t", mode, type,
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 126e7097bf6..d94fd170243 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1863,9 +1863,10 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
 			die("object %s cannot be read",
 			    oid_to_hex(&trg_entry->idx.oid));
 		if (sz != trg_size)
-			die("object %s inconsistent object length (%lu vs %lu)",
-			    oid_to_hex(&trg_entry->idx.oid), sz,
-			    trg_size);
+			die("object %s inconsistent object length (%" PRIuMAX
+			    " vs %" PRIuMAX ")",
+			    oid_to_hex(&trg_entry->idx.oid), (uintmax_t)sz,
+			    (uintmax_t)trg_size);
 		*mem_usage += sz;
 	}
 	if (!src->data) {
@@ -1891,9 +1892,10 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
 			    oid_to_hex(&src_entry->idx.oid));
 		}
 		if (sz != src_size)
-			die("object %s inconsistent object length (%lu vs %lu)",
-			    oid_to_hex(&src_entry->idx.oid), sz,
-			    src_size);
+			die("object %s inconsistent object length (%" PRIuMAX
+			    " vs %" PRIuMAX ")",
+			    oid_to_hex(&src_entry->idx.oid), (uintmax_t)sz,
+			    (uintmax_t)src_size);
 		*mem_usage += sz;
 	}
 	if (!src->index) {
diff --git a/diff.c b/diff.c
index 6084487a2fc..1e815e87acb 100644
--- a/diff.c
+++ b/diff.c
@@ -2971,7 +2971,7 @@ static void emit_binary_diff_body(struct diff_options *o,
 	}
 
 	if (delta && delta_size < deflate_size) {
-		char *s = xstrfmt("%lu", orig_size);
+		char *s = xstrfmt("%" PRIuMAX, (uintmax_t)orig_size);
 		emit_diff_symbol(o, DIFF_SYMBOL_BINARY_DIFF_HEADER_DELTA,
 				 s, strlen(s), 0);
 		free(s);
diff --git a/fast-import.c b/fast-import.c
index 011da200cb6..bfde330f215 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2986,8 +2986,8 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)
 		die("Object %s is a %s but a blob was expected.",
 		    oid_to_hex(oid), typename(type));
 	strbuf_reset(&line);
-	strbuf_addf(&line, "%s %s %lu\n", oid_to_hex(oid),
-						typename(type), size);
+	strbuf_addf(&line, "%s %s %" PRIuMAX "\n", oid_to_hex(oid),
+		    typename(type), (uintmax_t)size);
 	cat_blob_write(line.buf, line.len);
 	strbuf_release(&line);
 	cat_blob_write(buf, size);
diff --git a/http-push.c b/http-push.c
index 676149ceac3..af229428c1c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -356,7 +356,8 @@ static void start_put(struct transfer_request *request)
 	git_zstream stream;
 
 	unpacked = read_sha1_file(request->obj->oid.hash, &type, &len);
-	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
+	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %" PRIuMAX,
+			   typename(type), (uintmax_t)len) + 1;
 
 	/* Set it up */
 	git_deflate_init(&stream, zlib_compression_level);
diff --git a/sha1_file.c b/sha1_file.c
index c63f478a311..84b9c6b6ec8 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -983,7 +983,8 @@ int check_sha1_signature(const unsigned char *sha1, void *map,
 		return -1;
 
 	/* Generate the header */
-	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(obj_type), size) + 1;
+	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %" PRIuMAX,
+			   typename(obj_type), (uintmax_t)size) + 1;
 
 	/* Sha1.. */
 	git_SHA1_Init(&c);
@@ -2794,7 +2795,8 @@ int force_object_loose(const unsigned char *sha1, time_t mtime)
 	buf = read_packed_sha1(sha1, &type, &len);
 	if (!buf)
 		return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
-	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
+	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %" PRIuMAX,
+			   typename(type), (uintmax_t)len) + 1;
 	ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
 	free(buf);