Re: [PATCH] Fix git to be (more) ANSI C99 compliant.

From: Junio C Hamano <junkio@cox.net>
Date: Jun 20, 2006, 01:59 UTC
Message-ID: <7vac8860z9.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20060619212116.GL1331@verplant.org>

Florian Forster <octo@verplant.org> writes:

> I didn't start writing the patch because I like C99 so much. In fact, in
> my opinion it introduces some possibilities I'd rather not have in C
> because people might actually use them. But by default the Sun cc
> complains about void-pointer arithmetic...

I am reasonably sympathetic to that, and judging from the number of lines the patch touches, I am not as strongly opposed to it as Linus seems to be.

> Maybe Rene Scharfe's method (as used in the patch to git-tar-tree) is a
> good way around it? There are no explicit casts involved and standard-
> compliant compilers like it, too. The downside is that you have two
> variables for the same thing/memory.

I think Rene's patch makes sense primarily because the functions affected are small and we can easily see that the aliased input variable ("data") is not used -- IOW, there is no confusion.

If it were a big function and the code used one variable for some purpose and the other one for another purpose, it would be far worse than having to cast the same variable occasionally.

BTW, I think we would probably want to have this patch on top of Rene's patch. In all instances, the variable "buf" is of type "const char *" and the existing casts do not make sense to me.

diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 5c8a5f0..39a61b6 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -36,7 +36,7 @@ static void reliable_write(const void *d
 			die("git-tar-tree: disk full?");
 		}
 		size -= ret;
-		buf = (char *) buf + ret;
+		buf += ret;
 	}
 }
 
@@ -65,13 +65,13 @@ static void write_blocked(const void *da
 		memcpy(block + offset, buf, chunk);
 		size -= chunk;
 		offset += chunk;
-		buf = (char *) buf + chunk;
+		buf += chunk;
 		write_if_needed();
 	}
 	while (size >= BLOCKSIZE) {
 		reliable_write(buf, BLOCKSIZE);
 		size -= BLOCKSIZE;
-		buf = (char *) buf + BLOCKSIZE;
+		buf += BLOCKSIZE;
 	}
 	if (size) {
 		memcpy(block + offset, buf, size);