[PATCH] Avoid C++ comments, use C comments instead

[PATCH] Avoid C++ comments, use C comments instead

[Pavel Roskin]

From: Pavel Roskin <proski@gnu.org>

This doesn't make the code uglier or harder to read, yet it makes the
code more portable.  This also simplifies checking for other potential
incompatibilities.  "gcc -std=c89 -pedantic" can flag many incompatible
constructs as warnings, but C++ comments will cause it to emit an error.

Signed-off-by: Pavel Roskin <proski@gnu.org>
---

 blame.c           |    6 +++---
 builtin-apply.c   |   21 +++++++++++----------
 builtin-push.c    |    2 +-
 convert-objects.c |   10 +++++-----
 dir.c             |    2 +-
 http-fetch.c      |    6 +++---
 mktag.c           |    5 +++--
 read-cache.c      |    2 +-
 sha1_file.c       |    4 ++--
 ssh-fetch.c       |    4 ++--
 10 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/blame.c b/blame.c
index 0a06026..b04b8f5 100644
--- a/blame.c
+++ b/blame.c
@@ -44,8 +44,8 @@ struct util_info {
 };
 
 struct chunk {
-	int off1, len1;	// ---
-	int off2, len2;	// +++
+	int off1, len1;	/* --- */
+	int off2, len2;	/* +++ */
 };
 
 struct patch {
@@ -255,7 +255,7 @@ static void print_map(struct commit *cmi
 }
 #endif
 
-// p is a patch from commit to other.
+/* p is a patch from commit to other. */
 static void fill_line_map(struct commit *commit, struct commit *other,
 			  struct patch *p)
 {
diff --git a/builtin-apply.c b/builtin-apply.c
index 1e5b846..c903146 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -14,14 +14,15 @@ #include "blob.h"
 #include "delta.h"
 #include "builtin.h"
 
-//  --check turns on checking that the working tree matches the
-//    files that are being modified, but doesn't apply the patch
-//  --stat does just a diffstat, and doesn't actually apply
-//  --numstat does numeric diffstat, and doesn't actually apply
-//  --index-info shows the old and new index info for paths if available.
-//  --index updates the cache as well.
-//  --cached updates only the cache without ever touching the working tree.
-//
+/*
+ *  --check turns on checking that the working tree matches the
+ *    files that are being modified, but doesn't apply the patch
+ *  --stat does just a diffstat, and doesn't actually apply
+ *  --numstat does numeric diffstat, and doesn't actually apply
+ *  --index-info shows the old and new index info for paths if available.
+ *  --index updates the cache as well.
+ *  --cached updates only the cache without ever touching the working tree.
+ */
 static const char *prefix;
 static int prefix_length = -1;
 static int newfd = -1;
@@ -284,8 +285,8 @@ static void parse_traditional_patch(cons
 {
 	char *name;
 
-	first += 4;	// skip "--- "
-	second += 4;	// skip "+++ "
+	first += 4;	/* skip "--- " */
+	second += 4;	/* skip "+++ " */
 	if (is_dev_null(first)) {
 		patch->is_new = 1;
 		patch->is_delete = 0;
diff --git a/builtin-push.c b/builtin-push.c
index a8fac88..31cbfd7 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -273,7 +273,7 @@ static int do_push(const char *repo)
 int cmd_push(int argc, const char **argv, char **envp)
 {
 	int i;
-	const char *repo = "origin";	// default repository
+	const char *repo = "origin";	/* default repository */
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/convert-objects.c b/convert-objects.c
index 0fabd89..478571f 100644
--- a/convert-objects.c
+++ b/convert-objects.c
@@ -241,13 +241,13 @@ static void convert_date(void *buffer, u
 	char *new = xmalloc(size + 100);
 	unsigned long newlen = 0;
 	
-	// "tree <sha1>\n"
+	/* "tree <sha1>\n" */
 	memcpy(new + newlen, buffer, 46);
 	newlen += 46;
 	buffer = (char *) buffer + 46;
 	size -= 46;
 
-	// "parent <sha1>\n"
+	/* "parent <sha1>\n" */
 	while (!memcmp(buffer, "parent ", 7)) {
 		memcpy(new + newlen, buffer, 48);
 		newlen += 48;
@@ -255,12 +255,12 @@ static void convert_date(void *buffer, u
 		size -= 48;
 	}
 
-	// "author xyz <xyz> date"
+	/* "author xyz <xyz> date" */
 	newlen += convert_date_line(new + newlen, &buffer, &size);
-	// "committer xyz <xyz> date"
+	/* "committer xyz <xyz> date" */
 	newlen += convert_date_line(new + newlen, &buffer, &size);
 
-	// Rest
+	/* Rest */
 	memcpy(new + newlen, buffer, size);
 	newlen += size;
 
diff --git a/dir.c b/dir.c
index d778ecd..092d077 100644
--- a/dir.c
+++ b/dir.c
@@ -336,7 +336,7 @@ static int read_directory_recursive(stru
 				if (dir->show_other_directories &&
 				    (subdir || !dir->hide_empty_directories) &&
 				    !dir_exists(fullname, baselen + len)) {
-					// Rewind the read subdirectory
+					/* Rewind the read subdirectory */
 					while (dir->nr > rewind_base)
 						free(dir->entries[--dir->nr]);
 					break;
diff --git a/http-fetch.c b/http-fetch.c
index 44eba5f..12493fb 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -490,7 +490,7 @@ static int setup_index(struct alt_base *
 {
 	struct packed_git *new_pack;
 	if (has_pack_file(sha1))
-		return 0; // don't list this as something we can get
+		return 0; /* don't list this as something we can get */
 
 	if (fetch_index(repo, sha1))
 		return -1;
@@ -570,7 +570,7 @@ static void process_alternates_response(
 						 base[serverlen - 1] != '/');
 					i += 3;
 				}
-				// If the server got removed, give up.
+				/* If the server got removed, give up. */
 				okay = strchr(base, ':') - base + 3 <
 					serverlen;
 			} else if (alt_req->http_specific) {
@@ -581,7 +581,7 @@ static void process_alternates_response(
 					okay = 1;
 				}
 			}
-			// skip 'objects' at end
+			/* skip 'objects' at end */
 			if (okay) {
 				target = xmalloc(serverlen + posn - i - 6);
 				strlcpy(target, base, serverlen);
diff --git a/mktag.c b/mktag.c
index f0fe528..27f4c4f 100644
--- a/mktag.c
+++ b/mktag.c
@@ -17,7 +17,7 @@ #include "tag.h"
  * in that size, you're doing something wrong.
  */
 
-// Some random size
+/* Some random size */
 #define MAXSIZE (8192)
 
 /*
@@ -123,7 +123,8 @@ int main(int argc, char **argv)
 		die("could not read from stdin");
 	}
 
-	// Verify it for some basic sanity: it needs to start with "object <sha1>\ntype\ntagger "
+	/* Verify it for some basic sanity: it needs to start with
+	   "object <sha1>\ntype\ntagger " */
 	if (verify_tag(buffer, size) < 0)
 		die("invalid tag signature file");
 
diff --git a/read-cache.c b/read-cache.c
index 3c32aae..a50d361 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -748,7 +748,7 @@ int read_cache(void)
 		die("index file open failed (%s)", strerror(errno));
 	}
 
-	size = 0; // avoid gcc warning
+	size = 0; /* avoid gcc warning */
 	map = MAP_FAILED;
 	if (!fstat(fd, &st)) {
 		size = st.st_size;
diff --git a/sha1_file.c b/sha1_file.c
index f7bb3a1..459430a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -453,7 +453,7 @@ int use_packed_git(struct packed_git *p)
 {
 	if (!p->pack_size) {
 		struct stat st;
-		// We created the struct before we had the pack
+		/* We created the struct before we had the pack */
 		stat(p->pack_name, &st);
 		if (!S_ISREG(st.st_mode))
 			die("packfile %s not a regular file", p->pack_name);
@@ -1504,7 +1504,7 @@ static void *repack_object(const unsigne
 	int hdrlen;
 	void *buf;
 
-	// need to unpack and recompress it by itself
+	/* need to unpack and recompress it by itself */
 	unpacked = read_packed_sha1(sha1, type, &len);
 
 	hdrlen = sprintf(hdr, "%s %lu", type, len) + 1;
diff --git a/ssh-fetch.c b/ssh-fetch.c
index 1e59cd2..28f7fd9 100644
--- a/ssh-fetch.c
+++ b/ssh-fetch.c
@@ -68,7 +68,7 @@ int fetch(unsigned char *sha1)
 	struct object_list *temp;
 
 	if (memcmp(sha1, in_transit->item->sha1, 20)) {
-		// we must have already fetched it to clean the queue
+		/* we must have already fetched it to clean the queue */
 		return has_sha1_file(sha1) ? 0 : -1;
 	}
 	prefetches--;
@@ -85,7 +85,7 @@ int fetch(unsigned char *sha1)
 		if (read(fd_in, &remote, 1) < 1)
 			return -1;
 	}
-	//fprintf(stderr, "Got %d\n", remote);
+	/* fprintf(stderr, "Got %d\n", remote); */
 	if (remote < 0)
 		return remote;
 	ret = write_sha1_from_fd(sha1, fd_in, conn_buf, 4096, &conn_buf_posn);

Re: [PATCH] Avoid C++ comments, use C comments instead

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> From: Pavel Roskin <proski@gnu.org>
>
> This doesn't make the code uglier or harder to read, yet it makes the
> code more portable.  This also simplifies checking for other potential
> incompatibilities.  "gcc -std=c89 -pedantic" can flag many incompatible
> constructs as warnings, but C++ comments will cause it to emit an error.
>
> Signed-off-by: Pavel Roskin <proski@gnu.org>

The title should probably read "avoid C99 comments", but coming
from the previous century, I tend to agree with this.

The struct/array initializer stuff by Shawn makes them harder to
read (for structs, it moves initialization to actual code) and
more error prone (for arrays, now the initializers need to be
carefully kept ordered), but we do not have too many of them in
the code, so I do not think it is a not much of a practical
problem.  It is sad that some people stay behind and we need to
cater to them, though.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Olivier Galibert]

On Mon, Jul 10, 2006 at 12:46:28AM -0700, Junio C Hamano wrote:
> It is sad that some people stay behind and we need to
> cater to them, though.

Do you, really?

  OG.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Johannes Schindelin]

Hi,

On Mon, 10 Jul 2006, Olivier Galibert wrote:

> On Mon, Jul 10, 2006 at 12:46:28AM -0700, Junio C Hamano wrote:
> > It is sad that some people stay behind and we need to
> > cater to them, though.
> 
> Do you, really?

Well, I guess as long as things do not break for _you_, we do not need to.

Ciao,
Dscho

Re: [PATCH] Avoid C++ comments, use C comments instead

[Olivier Galibert]

On Mon, Jul 10, 2006 at 01:06:44PM +0200, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 10 Jul 2006, Olivier Galibert wrote:
> 
> > On Mon, Jul 10, 2006 at 12:46:28AM -0700, Junio C Hamano wrote:
> > > It is sad that some people stay behind and we need to
> > > cater to them, though.
> > 
> > Do you, really?
> 
> Well, I guess as long as things do not break for _you_, we do not need to.

Supporting old, not-standard-anymore compilers has a cost in
maintainability, by precluding the use of better constructs (//
comments, declarations near initialisation, struct initializers...).
Additionally, it gets harder and harder to have people test for them.
Given than you can find gcc on pretty much everything that has a
filesystem cache decent enough to handle git correctly, is this cost
worth it?  _That_ was the question.

  OG.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Paul Serice]

> Given than you can find gcc on pretty much everything that has a
> filesystem cache decent enough to handle git correctly, is this cost
> worth it?  _That_ was the question.

I've seen this argument before.  Unfortunately it seems reasonable
enough on the surface, and I actually bought into it much to may later
regret.

My experience is that gcc often produces buggy code, and if gcc is not
_the_ compiler for that platform, those bugs do not get fixed.
Specifically, I have had lots of problems with gcc and IRIX.

If you want to write portable code, you have to take into account
different operating systems _and_ different compilers.  Writing your
code for just a single compiler is almost as bad as writing your code
for just a single operating system.

Paul Serice

Re: [PATCH] Avoid C++ comments, use C comments instead

[Olivier Galibert]

On Mon, Jul 10, 2006 at 02:14:17PM -0500, Paul Serice wrote:
> If you want to write portable code, you have to take into account
> different operating systems _and_ different compilers.  Writing your
> code for just a single compiler is almost as bad as writing your code
> for just a single operating system.

Hmmm, that was not so much about gcc-specific code than which kind of
C you want to code to, the one from 1973, the one from 1989 or the one
from 1999?  I personally don't have much sympathy for the OS vendors
giving you an older standard C compiler and selling you the up-to-date
one.

  OG.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Johannes Schindelin]

Hi,

On Mon, 10 Jul 2006, Olivier Galibert wrote:

> On Mon, Jul 10, 2006 at 02:14:17PM -0500, Paul Serice wrote:
> > If you want to write portable code, you have to take into account
> > different operating systems _and_ different compilers.  Writing your
> > code for just a single compiler is almost as bad as writing your code
> > for just a single operating system.
> 
> Hmmm, that was not so much about gcc-specific code than which kind of
> C you want to code to, the one from 1973, the one from 1989 or the one
> from 1999?  I personally don't have much sympathy for the OS vendors
> giving you an older standard C compiler and selling you the up-to-date
> one.

Judging by what you say, one could get the impression you'd have not much 
sympathy for people being stuck with non-C99 compilers.

Just look at it: if the OS vendor just does not _care_, and you blame the 
vendor for not providing something newer, the vendor does not _care_ about 
your complaint either. But the user does.

However, there is a more important point to be made. If you are complying 
with an older standard, you get more users. More users = more bug testers.

And there were quite a few occasions where I found bugs by trying to run 
on a different platform, which was less forgiving than Linux. These are 
bugs you have a harder time to spot on Linux, _because_ Linux is so nice. 
But they will surface. And they will be a PITA to find.

Anyway, it is best practice for a reason to program portably. (Well, at 
least if you are not living in Redmont.)

Ciao,
Dscho

Re: [PATCH] Avoid C++ comments, use C comments instead

[Yakov Lerner]

On 7/11/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 10 Jul 2006, Olivier Galibert wrote:
>
> > On Mon, Jul 10, 2006 at 02:14:17PM -0500, Paul Serice wrote:
> > > If you want to write portable code, you have to take into account
> > > different operating systems _and_ different compilers.  Writing your
> > > code for just a single compiler is almost as bad as writing your code
> > > for just a single operating system.
> >
> > Hmmm, that was not so much about gcc-specific code than which kind of
> > C you want to code to, the one from 1973, the one from 1989 or the one
> > from 1999?  I personally don't have much sympathy for the OS vendors
> > giving you an older standard C compiler and selling you the up-to-date
> > one.
>
> Judging by what you say, one could get the impression you'd have not much
> sympathy for people being stuck with non-C99 compilers.
>
> Just look at it: if the OS vendor just does not _care_, and you blame the
> vendor for not providing something newer, the vendor does not _care_ about
> your complaint either. But the user does.
>
> However, there is a more important point to be made. If you are complying
> with an older standard, you get more users. More users = more bug testers.
>
> And there were quite a few occasions where I found bugs by trying to run
> on a different platform, which was less forgiving than Linux. These are
> bugs you have a harder time to spot on Linux, _because_ Linux is so nice.
> But they will surface. And they will be a PITA to find.
>
> Anyway, it is best practice for a reason to program portably. (Well, at
> least if you are not living in Redmont.)

Back in the beginning of nineties, c89 was new and the
prototypes was not yet impemented on many compilers.
One good trick of the time was automatic
source conversion. This protoize/unprotoize tool converted
sources from non-prototyped form to
prototyped, and the other way. (There was also #ifdef-trick
which was ugly as hell for the same purpose, and the __P()
macro trick, which was less ugly).

The benefit was that you got the benefit of both worlds,
(1) the benefit of prototypes when compiler was c89-compliant, and
(2) compilability with pre-c89 compilers when needed.

I am writing in order to ask, whether there maybe
some c99-to-c89 source convertor that can be
automatically applied to the .c before compiling with
pre-c99 compiler ?

Yakov

Re: [PATCH] Avoid C++ comments, use C comments instead

[Olivier Galibert]

On Tue, Jul 11, 2006 at 12:55:59AM +0200, Johannes Schindelin wrote:
> Judging by what you say, one could get the impression you'd have not much 
> sympathy for people being stuck with non-C99 compilers.

More and more these people are stuck with such a compiler because they
want to be.


> Just look at it: if the OS vendor just does not _care_, and you blame the 
> vendor for not providing something newer, the vendor does not _care_ about 
> your complaint either. But the user does.

Well, I'm not talking about git here, but I'm not really interested in
making my code harder to maintain just to provide more value for said
vendor.


> However, there is a more important point to be made. If you are complying 
> with an older standard, you get more users. More users = more bug testers.

On such systems, what you tend to find is bugs of the system, or
simply different, and sometimes somewhat nonsensical, implementations
of some compromise-happy standards like POSIX.


> And there were quite a few occasions where I found bugs by trying to run 
> on a different platform, which was less forgiving than Linux. These are 
> bugs you have a harder time to spot on Linux, _because_ Linux is so nice. 
> But they will surface. And they will be a PITA to find.

And how many bugs or "features" of the platform did you have to code
around before finding a genuine bug?


> Anyway, it is best practice for a reason to program portably. (Well, at 
> least if you are not living in Redmont.)

If "programming portably" meant catering to the oldest standards, then
you shouldn't even use prototypes or ansi-style function declarations.
After all, some people may be stuck with the old sun (or was it hp?)
compiler that was k&r only.  And limit the filenames to 14 characters.
Also, you shouldn't require gtk, python, perl, fire or the wheel.

  OG.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Olivier Galibert]

On Tue, Jul 11, 2006 at 02:25:44AM +0300, Yakov Lerner wrote:
> I am writing in order to ask, whether there maybe
> some c99-to-c89 source convertor that can be
> automatically applied to the .c before compiling with
> pre-c99 compiler ?

Comments are easy.  Moving declarations without breaking initializers
is harder.  Rewriting the struct initializers is pretty much
impossible without the tool turning into a full-blown C parser.

Maybe git can be perfectly happy with c89.  I don't know.  I know the
linux kernel requires c99, mostly for the struct initializers.  My
point was that staying at the c89 level has a maintainance cost, and a
cost/benefit analysis should be done to decide whether it is a good
idea.  Answering "get a C compiler", as is being done for some years
now for people not wanting prototypes, is an option not to neglect.

  OG.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Shawn Pearce]

Olivier Galibert <galibert@pobox.com> wrote:
> On Tue, Jul 11, 2006 at 02:25:44AM +0300, Yakov Lerner wrote:
> > I am writing in order to ask, whether there maybe
> > some c99-to-c89 source convertor that can be
> > automatically applied to the .c before compiling with
> > pre-c99 compiler ?
> 
> Comments are easy.  Moving declarations without breaking initializers
> is harder.  Rewriting the struct initializers is pretty much
> impossible without the tool turning into a full-blown C parser.
> 
> Maybe git can be perfectly happy with c89.  I don't know.  I know the
> linux kernel requires c99, mostly for the struct initializers.  My
> point was that staying at the c89 level has a maintainance cost, and a
> cost/benefit analysis should be done to decide whether it is a good
> idea.  Answering "get a C compiler", as is being done for some years
> now for people not wanting prototypes, is an option not to neglect.

GIT 1.2.3 had a lot more struct initializers than GIT 1.4.1 has.  So
apparently it was cleaner to remove a few of them in some cases then
it was to keep them in.  But that's besides the point.

I can understand the core maintainers not wanting to apply my patch
and lose the benefits of c99, and if I have to I'll carry a private
branch with that patch and hand-edit future versions as necessary
to get the same result...  but I'd hate to see another user have
to do the same work for the same reason.

I'm not a big contributor to GIT (I certainly don't contribute
nearly as much code as most others) and I'm also not a big user of
GIT (I don't develop for the Linux kernel) so I not expecting the
core to drop to c89 just for me and this old compiler.  :-)

After reading this thread I'm thinking that this probably shouldn't
get merged in and that I should carry the tweaks locally to get
GIT to build on the only compiler I have available on that system.
Now that GIT 1.4.1 is installed on there I'm unlikely to upgrade it
for at least 6 months, as I'm using only the very low level plumbing
(git-read-tree, git-write-tree, git-update-index, git-repack).
Remerging these c99 downgrades at that time shouldn't be a huge
issue for me since its probably going to be done so infrequently.

-- 
Shawn.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> Olivier Galibert <galibert@pobox.com> wrote:
>>
>> Maybe git can be perfectly happy with c89.  I don't know.  I know the
>> linux kernel requires c99, mostly for the struct initializers.  My
>> point was that staying at the c89 level has a maintainance cost, and a
>> cost/benefit analysis should be done to decide whether it is a good
>> idea.

I am generally in favor of the effect C99 struct and array
initializers have on the readability, but we also need to
balance that with the reality.  The patch is only about a few
structs and one array if I recall correctly isn't it?

What Olivier says is perfectly correct, and after "cost/benefit
analysis", I would have to say avoiding some C99 is fine if that
makes people's life on some major non-Linux platforms easier.

C99 clean-up is already in the "master", so please do not waste
more bandwidth nor time on this issue, but instead spend time
elsewhere to make our system better for more people ;-).

Thanks.

Re: [PATCH] Avoid C++ comments, use C comments instead

[Pavel Roskin]

Hello!

On Mon, 2006-07-10 at 13:41 +0200, Olivier Galibert wrote:
> Supporting old, not-standard-anymore compilers has a cost in
> maintainability, by precluding the use of better constructs (//
> comments, declarations near initialisation, struct initializers...).
> Additionally, it gets harder and harder to have people test for them.

Sorry for one more addition to this thread.  I just want to clear some
misunderstanding.  The whole point of fixing the comments is to make is
easier to test for other compatibility issues using gcc.

For gcc to report post-c89 features, "-pedantic -std=c89" should be
supplied.  This option makes gcc report the c99 comments as errors and
other c99 features as warnings.  The errors would stand in the way of
finding the warnings.

I'm not saying all non-c89 constructs should be fixed, but if we get a
report that some feature is not working with some compiler, we could
compile git with "-pedantic -std=c89", find corresponding warnings and
fix them.  The comments would stand in the way for somebody using gcc.

-- 
Regards,
Pavel Roskin