Re: [PATCH] Fix git to be (more) ANSI C99 compliant.
- From
Junio C Hamano <junkio@cox.net>
- Date
- Jun 18, 2006, 08:29 UTC
- Message-ID
- <7vac8a50ji.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <1150609831500-git-send-email-octo@verplant.org>
Florian Forster <octo@verplant.org> 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.
Show 10 quoted lines
> 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 "=".
Show 33 quoted lines
> @@ -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.
Show 19 quoted lines
> 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.
Show 12 quoted lines
> 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.
Show 19 quoted lines
> @@ -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-<.
Show 7 quoted lines
> 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?