Re: [PATCH] Fix git to be (more) ANSI C99 compliant.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:30
Florian Forster [off-list ref] writes:
While most of this patch fixes void-pointer arithmetic and is therefore trivial, I had to change the use of a struct with FAMs in `diff-lib.c'. Since this is the first time I encountered FAMs it'd probably be a good idea if someone who knows would take a look at that.
Thanks. I am very tempted to apply it, but I started to wonder that in some places it might make sense to convert void* to char* instead of casting. Undecided.
quoted hunk
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c index f6310b9..646322d 100644 --- a/builtin-tar-tree.c +++ b/builtin-tar-tree.c@@ -34,7 +34,7 @@ static void reliable_write(void *buf, un die("git-tar-tree: disk full?"); } size -= ret; - buf += ret; + buf = (char *) buf + ret;
Please do not add the extra whitespace to align "=".
quoted hunk
@@ -244,14 +244,14 @@ static void convert_date(void *buffer, u // "tree <sha1>\n" memcpy(new + newlen, buffer, 46); newlen += 46; - buffer += 46; + buffer = (char *) buffer + 46; size -= 46; // "parent <sha1>\n" while (!memcmp(buffer, "parent ", 7)) { memcpy(new + newlen, buffer, 48); newlen += 48; - buffer += 48; + buffer = (char *) buffer + 48; size -= 48; }@@ -275,11 +275,11 @@ static void convert_commit(void *buffer, if (memcmp(buffer, "tree ", 5)) die("Bad commit '%s'", (char*) buffer); - convert_ascii_sha1(buffer+5); - buffer += 46; /* "tree " + "hex sha1" + "\n" */ + convert_ascii_sha1((char *) buffer + 5); + buffer = (char *) buffer + 46; /* "tree " + "hex sha1" + "\n" */ while (!memcmp(buffer, "parent ", 7)) { - convert_ascii_sha1(buffer+7); - buffer += 48; + convert_ascii_sha1((char *) buffer + 7); + buffer = (char *) buffer + 48; } convert_date(orig_buffer, orig_size, result_sha1); }
Hmmmmmmm. Now I start to wonder if changing the type of "void *buffer" to "char *buffer" is cleaner.
quoted hunk
diff --git a/diff-delta.c b/diff-delta.c index 25a798d..8b9172a 100644 --- a/diff-delta.c +++ b/diff-delta.c@@ -22,6 +22,7 @@ #include <stdlib.h> #include <string.h> #include "delta.h" +#include "git-compat-util.h" /* maximum hash entry list for the same hash bucket */ #define HASH_LIMIT 64@@ -131,7 +132,7 @@ struct delta_index { const void *src_buf; unsigned long src_size; unsigned int hash_mask; - struct index_entry *hash[0]; + struct index_entry *hash[FLEX_ARRAY]; };
Good -- I missed this when we did FLEX_ARRAY. Thanks.
quoted hunk
diff --git a/diff-lib.c b/diff-lib.c index 2183b41..fdc1173 100644 --- a/diff-lib.c +++ b/diff-lib.c@@ -34,21 +34,23 @@ int run_diff_files(struct rev_info *revs continue; if (ce_stage(ce)) { - struct { - struct combine_diff_path p; - struct combine_diff_parent filler[5]; - } combine;
I admit this part was ugly. The new code does not do any extra allocations and matches the other use of "struct combine_diff_path" more closely. Good change.
quoted hunk
@@ -1136,13 +1136,14 @@ int fetch(unsigned char *sha1) static inline int needs_quote(int ch) { - switch (ch) { - case '/': case '-': case '.': - case 'A'...'Z': case 'a'...'z': case '0'...'9': + if (((ch >= 'A') && (ch <= 'Z')) + || ((ch >= 'a') && (ch <= 'z')) + || ((ch >= '0') && (ch <= '9')) + || (ch == '/') + || (ch == '-') + || (ch == '.')) return 0; - default: - return 1; - } + return 1; }
Ugh. Delight of standard compliance X-<.
quoted hunk
diff --git a/http-push.c b/http-push.c index 2d9441e..0684e46 100644 --- a/http-push.c +++ b/http-push.c@@ -1077,13 +1077,14 @@ static int fetch_indices(void) static inline int needs_quote(int ch)
Hmph. Thanks for noticing the duplicated code; maybe move it to cache.h perhaps?