git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/4] archive-tar: mark RECORDSIZE/BLOCKSIZE as unsigned

From
René Scharfe <l.s.r@web.de>
Date
May 10, 2019, 17:18 UTC
Message-ID
<f380b182-633f-f347-ed2e-f90548d7f3c9@web.de>
In-Reply-To
<20190509183855.GA28107@sigill.intra.peff.net>
Am 09.05.19 um 20:38 schrieb Jeff King:
Show 6 quoted lines
> I do dream of a world where we do not have a bunch of implicit
> conversions (both signedness but also truncation) in our code base, and
> can compile cleanly with -Wconversion We know that this case is
> perfectly fine, but I am sure there are many that are not. However, I'm
> not sure if we'll ever get there, and in the meantime I don't think it's
> worth worrying too much about individual cases like this.

Here's a rough take on how to silence that warning for archive-tar.c using GCC 8.3. Some of the changes are worth polishing and submitting. Some are silly. The one for regexec_buf() is scary; I don't see a clean way of dealing with that size_t to int conversion.

---
 archive-tar.c     | 54 +++++++++++++++++++++++++++++++----------------
 cache.h           | 10 ++++-----
 git-compat-util.h | 21 +++++++++++++++---
 hash.h            |  2 +-
 strbuf.h          |  2 +-
 5 files changed, 61 insertions(+), 28 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 3e53aac1e6..bfd91782ab 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -15,7 +15,7 @@
 static char block[BLOCKSIZE];
 static unsigned long offset;

-static int tar_umask = 002;
+static mode_t tar_umask = 002;

 static int write_tar_filter_archive(const struct archiver *ar,
 				    struct archiver_args *args);
@@ -99,7 +99,7 @@ static void write_blocked(const void *data, unsigned long size)
  */
 static void write_trailer(void)
 {
-	int tail = BLOCKSIZE - offset;
+	size_t tail = BLOCKSIZE - offset;
 	memset(block + offset, 0, tail);
 	write_or_die(1, block, BLOCKSIZE);
 	if (tail < 2 * RECORDSIZE) {
@@ -127,12 +127,13 @@ static int stream_blocked(const struct object_id *oid)
 		readlen = read_istream(st, buf, sizeof(buf));
 		if (readlen <= 0)
 			break;
-		do_write_blocked(buf, readlen);
+		do_write_blocked(buf, (size_t)readlen);
 	}
 	close_istream(st);
-	if (!readlen)
-		finish_record();
-	return readlen;
+	if (readlen < 0)
+		return -1;
+	finish_record();
+	return 0;
 }

 /*
@@ -142,9 +143,9 @@ static int stream_blocked(const struct object_id *oid)
  * string and appends it to a struct strbuf.
  */
 static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
-				     const char *value, unsigned int valuelen)
+				     const char *value, size_t valuelen)
 {
-	int len, tmp;
+	size_t len, tmp;

 	/* "%u %s=%s\n" */
 	len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1;
@@ -152,7 +153,7 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
 		len++;

 	strbuf_grow(sb, len);
-	strbuf_addf(sb, "%u %s=", len, keyword);
+	strbuf_addf(sb, "%"PRIuMAX" %s=", (uintmax_t)len, keyword);
 	strbuf_add(sb, value, valuelen);
 	strbuf_addch(sb, '\n');
 }
@@ -168,7 +169,9 @@ static void strbuf_append_ext_header_uint(struct strbuf *sb,
 	int len;

 	len = xsnprintf(buf, sizeof(buf), "%"PRIuMAX, value);
-	strbuf_append_ext_header(sb, keyword, buf, len);
+	if (len < 0)
+		BUG("unable to convert %"PRIuMAX" to decimal", value);
+	strbuf_append_ext_header(sb, keyword, buf, (size_t)len);
 }

 static unsigned int ustar_header_chksum(const struct ustar_header *header)
@@ -177,7 +180,7 @@ static unsigned int ustar_header_chksum(const struct ustar_header *header)
 	unsigned int chksum = 0;
 	while (p < (const unsigned char *)header->chksum)
 		chksum += *p++;
-	chksum += sizeof(header->chksum) * ' ';
+	chksum += (unsigned int)sizeof(header->chksum) * ' ';
 	p += sizeof(header->chksum);
 	while (p < (const unsigned char *)header + sizeof(struct ustar_header))
 		chksum += *p++;
@@ -355,12 +358,14 @@ static void write_global_extended_header(struct archiver_args *args)
 }

 static struct archiver **tar_filters;
-static int nr_tar_filters;
-static int alloc_tar_filters;
+static size_t nr_tar_filters;
+static size_t alloc_tar_filters;

-static struct archiver *find_tar_filter(const char *name, int len)
+static struct archiver *find_tar_filter(const char *name, size_t len)
 {
 	int i;
+	if (len < 1)
+		return NULL;
 	for (i = 0; i < nr_tar_filters; i++) {
 		struct archiver *ar = tar_filters[i];
 		if (!strncmp(ar->name, name, len) && !ar->name[len])
@@ -369,14 +374,27 @@ static struct archiver *find_tar_filter(const char *name, int len)
 	return NULL;
 }

+static int parse_config_key2(const char *var, const char *section,
+			     const char **subsection, size_t *subsection_len,
+			     const char **key)
+{
+	int rc, len;
+
+	rc = parse_config_key(var, section, subsection, &len, key);
+	if (!rc && len < 0)
+		return -1;
+	*subsection_len = (size_t)len;
+	return rc;
+}
+
 static int tar_filter_config(const char *var, const char *value, void *data)
 {
 	struct archiver *ar;
 	const char *name;
 	const char *type;
-	int namelen;
+	size_t namelen;

-	if (parse_config_key(var, "tar", &name, &namelen, &type) < 0 || !name)
+	if (parse_config_key2(var, "tar", &name, &namelen, &type) < 0 || !name)
 		return 0;

 	ar = find_tar_filter(name, namelen);
@@ -400,7 +418,7 @@ static int tar_filter_config(const char *var, const char *value, void *data)
 		if (git_config_bool(var, value))
 			ar->flags |= ARCHIVER_REMOTE;
 		else
-			ar->flags &= ~ARCHIVER_REMOTE;
+			ar->flags &= ~(unsigned int)ARCHIVER_REMOTE;
 		return 0;
 	}

@@ -414,7 +432,7 @@ static int git_tar_config(const char *var, const char *value, void *cb)
 			tar_umask = umask(0);
 			umask(tar_umask);
 		} else {
-			tar_umask = git_config_int(var, value);
+			tar_umask = (mode_t)git_config_ulong(var, value);
 		}
 		return 0;
 	}
diff --git a/cache.h b/cache.h
index 67cc2e1806..a791034260 100644
--- a/cache.h
+++ b/cache.h
@@ -241,7 +241,7 @@ static inline void copy_cache_entry(struct cache_entry *dst,
 				    const struct cache_entry *src)
 {
 	unsigned int state = dst->ce_flags & CE_HASHED;
-	int mem_pool_allocated = dst->mem_pool_allocated;
+	unsigned int mem_pool_allocated = dst->mem_pool_allocated;

 	/* Don't copy hash chain and name */
 	memcpy(&dst->ce_stat_data, &src->ce_stat_data,
@@ -249,7 +249,7 @@ static inline void copy_cache_entry(struct cache_entry *dst,
 			offsetof(struct cache_entry, ce_stat_data));

 	/* Restore the hash state */
-	dst->ce_flags = (dst->ce_flags & ~CE_HASHED) | state;
+	dst->ce_flags = (dst->ce_flags & ~(unsigned int)CE_HASHED) | state;

 	/* Restore the mem_pool_allocated flag */
 	dst->mem_pool_allocated = mem_pool_allocated;
@@ -1314,7 +1314,7 @@ extern int check_and_freshen_file(const char *fn, int freshen);
 extern const signed char hexval_table[256];
 static inline unsigned int hexval(unsigned char c)
 {
-	return hexval_table[c];
+	return (unsigned int)hexval_table[c];
 }

 /*
@@ -1323,8 +1323,8 @@ static inline unsigned int hexval(unsigned char c)
  */
 static inline int hex2chr(const char *s)
 {
-	unsigned int val = hexval(s[0]);
-	return (val & ~0xf) ? val : (val << 4) | hexval(s[1]);
+	int val = hexval_table[(unsigned char)s[0]];
+	return (val < 0) ? val : (val << 4) | hexval_table[(unsigned char)s[1]];
 }

 /* Convert to/from hex/sha1 representation */
diff --git a/git-compat-util.h b/git-compat-util.h
index 4386b3e1c8..cf33e84c96 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1068,7 +1068,7 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
 	ul = strtoul(s, &p, base);
 	if (errno || *p || p == s || (unsigned int) ul != ul)
 		return -1;
-	*result = ul;
+	*result = (unsigned int)ul;
 	return 0;
 }

@@ -1081,7 +1081,7 @@ static inline int strtol_i(char const *s, int base, int *result)
 	ul = strtol(s, &p, base);
 	if (errno || *p || p == s || (int) ul != ul)
 		return -1;
-	*result = ul;
+	*result = (int)ul;
 	return 0;
 }

@@ -1119,7 +1119,22 @@ static inline int regexec_buf(const regex_t *preg, const char *buf, size_t size,
 {
 	assert(nmatch > 0 && pmatch);
 	pmatch[0].rm_so = 0;
-	pmatch[0].rm_eo = size;
+	pmatch[0].rm_eo = (regoff_t)size;
+	if (pmatch[0].rm_eo != size) {
+		if (((regoff_t)-1) < 0) {
+			if (sizeof(regoff_t) == sizeof(int))
+				pmatch[0].rm_eo = (regoff_t)INT_MAX;
+			else if (sizeof(regoff_t) == sizeof(long))
+				pmatch[0].rm_eo = (regoff_t)LONG_MAX;
+			else
+				die("unable to determine maximum value of regoff_t");
+		} else {
+			pmatch[0].rm_eo = (regoff_t)-1;
+		}
+		warning("buffer too big (%"PRIuMAX"), "
+			"will search only the first %"PRIuMAX" bytes",
+			(uintmax_t)size, (uintmax_t)pmatch[0].rm_eo);
+	}
 	return regexec(preg, buf, nmatch, pmatch, eflags | REG_STARTEND);
 }

diff --git a/hash.h b/hash.h
index 661c9f2281..7056f89eb4 100644
--- a/hash.h
+++ b/hash.h
@@ -134,7 +134,7 @@ int hash_algo_by_id(uint32_t format_id);
 /* Identical, except based on the length. */
 int hash_algo_by_length(int len);
 /* Identical, except for a pointer to struct git_hash_algo. */
-static inline int hash_algo_by_ptr(const struct git_hash_algo *p)
+static inline ptrdiff_t hash_algo_by_ptr(const struct git_hash_algo *p)
 {
 	return p - hash_algos;
 }
diff --git a/strbuf.h b/strbuf.h
index c8d98dfb95..30659f2d5d 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -225,7 +225,7 @@ int strbuf_cmp(const struct strbuf *first, const struct strbuf *second);
 /**
  * Add a single character to the buffer.
  */
-static inline void strbuf_addch(struct strbuf *sb, int c)
+static inline void strbuf_addch(struct strbuf *sb, char c)
 {
 	if (!strbuf_avail(sb))
 		strbuf_grow(sb, 1);
--
2.21.0
Previous: Jeff KingNext: Jeff King
Message 73 of 74 in “Avoid spawning gzip in git archive”
  1. 0/2 Avoid spawning gzip in git archiveJohannes Schindelin via GitGitGadget, Apr 12, 2019
  2. 1/2 archive: replace write_or_die() calls with write_block_or_die()Rohit Ashiwal via GitGitGadget, Apr 12, 2019
  3. Jeff KingApr 13, 2019
  4. Junio C HamanoApr 13, 2019
  5. Rohit AshiwalApr 14, 2019
  6. Johannes SchindelinApr 26, 2019
  7. Junio C HamanoApr 26, 2019
  8. Johannes SchindelinApr 29, 2019
  9. Jeff KingMay 1, 2019
  10. René ScharfeMay 2, 2019
  11. Junio C HamanoMay 5, 2019
  12. Jeff KingMay 6, 2019
  13. Rohit AshiwalApr 14, 2019
  14. Junio C HamanoApr 14, 2019
  15. Johannes SchindelinApr 26, 2019
  16. Jeff KingMay 1, 2019
  17. 2/2 archive: avoid spawning `gzip`Rohit Ashiwal via GitGitGadget, Apr 12, 2019
  18. Jeff KingApr 13, 2019
  19. René ScharfeApr 13, 2019
  20. Jeff KingApr 15, 2019
  21. Johannes SchindelinApr 26, 2019
  22. René ScharfeApr 27, 2019
  23. René ScharfeApr 27, 2019
  24. Johannes SchindelinApr 29, 2019
  25. René ScharfeMay 1, 2019
  26. Jeff KingMay 1, 2019
  27. René ScharfeJun 10, 2019
  28. Jeff KingJun 13, 2019
  29. brian m. carlsonApr 13, 2019
  30. Jeff KingApr 15, 2019
  31. Johannes SchindelinApr 26, 2019
  32. Ævar Arnfjörð BjarmasonMay 2, 2019
  33. Johannes SchindelinMay 3, 2019
  34. Jeff KingMay 3, 2019
  35. Johannes SchindelinApr 26, 2019
  36. 0/5 Avoid spawning gzip in git archiveRené Scharfe, Jun 12, 2022
  37. 1/5 archive: rename archiver data field to filter_commandRené Scharfe, Jun 12, 2022
  38. 2/5 archive-tar: factor out write_block()René Scharfe, Jun 12, 2022
  39. 3/5 archive-tar: add internal gzip implementationRené Scharfe, Jun 12, 2022
  40. Junio C HamanoJun 13, 2022
  41. 4/5 archive-tar: use OS_CODE 3 (Unix) for internal gzipRené Scharfe, Jun 12, 2022
  42. 5/5 archive-tar: use internal gzip by defaultRené Scharfe, Jun 12, 2022
  43. Junio C HamanoJun 13, 2022
  44. Johannes SchindelinJun 14, 2022
  45. René ScharfeJun 14, 2022
  46. René ScharfeJun 14, 2022
  47. Johannes SchindelinJun 14, 2022
  48. René ScharfeJun 14, 2022
  49. Junio C HamanoJun 15, 2022
  50. Johannes SchindelinJun 14, 2022
  51. René ScharfeJun 14, 2022
  52. Johannes SchindelinJun 30, 2022
  53. Johannes SchindelinJul 1, 2022
  54. Jeff KingJul 1, 2022
  55. Junio C HamanoJul 1, 2022
  56. 0/6 Avoid spawning gzip in git archiveRené Scharfe, Jun 15, 2022
  57. 1/6 archive: update format documentationRené Scharfe, Jun 15, 2022
  58. 2/6 archive: rename archiver data field to filter_commandRené Scharfe, Jun 15, 2022
  59. 3/6 archive-tar: factor out write_block()René Scharfe, Jun 15, 2022
  60. 4/6 archive-tar: add internal gzip implementationRené Scharfe, Jun 15, 2022
  61. Ævar Arnfjörð BjarmasonJun 15, 2022
  62. René ScharfeJun 16, 2022
  63. Ævar Arnfjörð BjarmasonJun 24, 2022
  64. René ScharfeJun 24, 2022
  65. 5/6 archive-tar: use OS_CODE 3 (Unix) for internal gzipRené Scharfe, Jun 15, 2022
  66. 6/6 archive-tar: use internal gzip by defaultRené Scharfe, Jun 15, 2022
  67. René ScharfeMay 2, 2019
  68. René ScharfeMay 2, 2019
  69. Johannes SchindelinMay 8, 2019
  70. Jeff KingMay 8, 2019
  71. Johannes SchindelinMay 9, 2019
  72. Jeff KingMay 9, 2019
  73. René ScharfeMay 10, 2019
  74. Jeff KingMay 10, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.