Issue with compiling git 1.8.4 under uclibc with gcc 4.6.3

Up until the latest release, I've been able to compile git for a uclibc based embedded linux. The toolchain I use is from "entware", which is based off of the openwrt toolchain.
https://code.google.com/p/wl500g-repo/

Somewhere between git 1.8.3.4 & 1.8.4 there seems to be some changes that breaks compilation for my particular situation. When cross compiling I receive the following error. I also receive the same error when compiling natively on the uclibc based device itself. Not sure if being uclibc based has anything to do with it, but is noteworthy. I am not a c programmer.
Both the cross compiler, and embedded device's version off gcc is 4.6.3. Previous versions compile fine

CC config.o
config.c: In function 'get_next_char':
config.c:220:14: error: expected identifier before '(' token config.c:220:14: error: expected statement before ')' token config.c:220:14: error: expected statement before ')' token config.c:224:11: error: expected identifier before '(' token

Lance
--
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 14 lines · Lance

Up until the latest release, I've been able to compile git for a uclibc based embedded linux. The toolchain I use is from "entware", which is based off of the openwrt toolchain.
https://code.google.com/p/wl500g-repo/

Somewhere between git 1.8.3.4 & 1.8.4 there seems to be some changes that breaks compilation for my particular situation. When cross compiling I receive the following error. I also receive the same error when compiling natively on the uclibc based device itself. Not sure if being uclibc based has anything to do with it, but is noteworthy. I am not a c programmer.
Both the cross compiler, and embedded device's version off gcc is 4.6.3. Previous versions compile fine

Quoted 6 lines

CC config.o
config.c: In function 'get_next_char':
config.c:220:14: error: expected identifier before '(' token config.c:220:14: error: expected statement before ')' token config.c:220:14: error: expected statement before ')' token config.c:224:11: error: expected identifier before '(' token

Does changing line 220 of config.c to

     int c = (cf->fgetc)(cf);

fix it?

Quoted 5 lines

Lance
--
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

[...]

Quoted 8 lines

Quoted 6 lines

CC config.o
config.c: In function 'get_next_char':
config.c:220:14: error: expected identifier before '(' token config.c:220:14: error: expected statement before ')' token config.c:220:14: error: expected statement before ')' token config.c:224:11: error: expected identifier before '(' token

Does changing line 220 of config.c to

Quoted 3 lines

    int c = (cf->fgetc)(cf);

fix it?

I also had to change line 224 to the following

                c = (cf->fgetc)(cf);

Once both places were changes, it compiled successfully.

[...]

Quoted 5 lines

I also had to change line 224 to the following

                c = (cf->fgetc)(cf);

Once both places were changes, it compiled successfully.

Sounds like a parser bug to me. Should we patch this in Git in order to make it compile with (broken?) GCC versions?

Hmm. I wonder if fgetc is a macro in uclibc? Just grepping their stdio.h, it looks like that is a possibility.

I think they are probably wrong to do so (but I'd have to check the standard). However, the cleaner workaround would probably be to call the fgetc struct member something else.

Quoted 6 lines · Jeff King

Hmm. I wonder if fgetc is a macro in uclibc? Just grepping their stdio.h, it looks like that is a possibility.

I think they are probably wrong to do so (but I'd have to check the standard). However, the cleaner workaround would probably be to call the fgetc struct member something else.

Nope, it's allowed. I think we should do the patch below:

-- >8 --
Subject: config: do not use C function names as struct members [...]

Instead, we can simply use non-colliding names.

Signed-off-by Jeff King <[email protected]>

---
 config.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

[...]

Quoted 6 lines · Jeff King

Hmm. I wonder if fgetc is a macro in uclibc? Just grepping their stdio.h, it looks like that is a possibility.

I think they are probably wrong to do so (but I'd have to check the standard). However, the cleaner workaround would probably be to call the fgetc struct member something else.

Nope, it's allowed. I think we should do the patch below:

-- >8 --
Subject: config: do not use C function names as struct members

According to C99, section 7.1.4:

Any function declared in a header may be additionally implemented as a function-like macro defined in the header.

Therefore calling our struct member function pointer "fgetc" may run afoul of unwanted macro expansion when we call:

  char c = cf->fgetc(cf);

This turned out to be a problem on uclibc, which defines fgetc as a macro and causes compilation failure.

The standard suggests fixing this in a few ways:

1. Using extra parentheses to inhibit the function-like macro expansion. E.g., "(cf->fgetc)(cf)". This is undesirable as it's ugly, and each call site needs to remember to use it (and on systems without the macro, forgetting will compile just fine).

2. Using #undef (because a conforming implementation must also be providing fgetc as a function). This is undesirable because presumably the implementation was using the macro for a performance benefit, and we are dropping that optimization.

Instead, we can simply use non-colliding names.

Signed-off-by Jeff King <[email protected]>

---
 config.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

config.c +16 -16

index e13a7b6..9f9bf0c 100644
@@ -27,9 +27,9 @@ struct config_source {
 	struct strbuf value;
 	struct strbuf var;
 
-	int (*fgetc)(struct config_source *c);
-	int (*ungetc)(int c, struct config_source *conf);
-	long (*ftell)(struct config_source *c);
+	int (*do_fgetc)(struct config_source *c);
+	int (*do_ungetc)(int c, struct config_source *conf);
+	long (*do_ftell)(struct config_source *c);
 };
 
 static struct config_source *cf;

@@ -217,13 +217,13 @@ static int get_next_char(void)
 
 static int get_next_char(void)
 {
-	int c = cf->fgetc(cf);
+	int c = cf->do_fgetc(cf);
 
 	if (c == '\r') {
 		/* DOS like systems */
-		c = cf->fgetc(cf);
+		c = cf->do_fgetc(cf);
 		if (c != '\n') {
-			cf->ungetc(c, cf);
+			cf->do_ungetc(c, cf);
 			c = '\r';
 		}
 	}

@@ -992,9 +992,9 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 		top.u.file = f;
 		top.name = filename;
 		top.die_on_error = 1;
-		top.fgetc = config_file_fgetc;
-		top.ungetc = config_file_ungetc;
-		top.ftell = config_file_ftell;
+		top.do_fgetc = config_file_fgetc;
+		top.do_ungetc = config_file_ungetc;
+		top.do_ftell = config_file_ftell;
 
 		ret = do_config_from(&top, fn, data);
 

@@ -1013,9 +1013,9 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,
 	top.u.buf.pos = 0;
 	top.name = name;
 	top.die_on_error = 0;
-	top.fgetc = config_buf_fgetc;
-	top.ungetc = config_buf_ungetc;
-	top.ftell = config_buf_ftell;
+	top.do_fgetc = config_buf_fgetc;
+	top.do_ungetc = config_buf_ungetc;
+	top.do_ftell = config_buf_ftell;
 
 	return do_config_from(&top, fn, data);
 }

@@ -1196,7 +1196,7 @@ static int store_aux(const char *key, const char *value, void *cb)
 				return 1;
 			}
 
-			store.offset[store.seen] = cf->ftell(cf);
+			store.offset[store.seen] = cf->do_ftell(cf);
 			store.seen++;
 		}
 		break;

@@ -1223,19 +1223,19 @@ static int store_aux(const char *key, const char *value, void *cb)
 		 * Do not increment matches: this is no match, but we
 		 * just made sure we are in the desired section.
 		 */
-		store.offset[store.seen] = cf->ftell(cf);
+		store.offset[store.seen] = cf->do_ftell(cf);
 		/* fallthru */
 	case SECTION_END_SEEN:
 	case START:
 		if (matches(key, value)) {
-			store.offset[store.seen] = cf->ftell(cf);
+			store.offset[store.seen] = cf->do_ftell(cf);
 			store.state = KEY_SEEN;
 			store.seen++;
 		} else {
 			if (strrchr(key, '.') - key == store.baselen &&
 			      !strncmp(key, store.key, store.baselen)) {
 					store.state = SECTION_SEEN;
-					store.offset[store.seen] = cf->ftell(cf);
+					store.offset[store.seen] = cf->do_ftell(cf);
 			}
 		}
 	}

Quoted 11 lines · Lance

[...]

Quoted 10 lines

Quoted 6 lines

CC config.o
config.c: In function 'get_next_char':
config.c:220:14: error: expected identifier before '(' token config.c:220:14: error: expected statement before ')' token config.c:220:14: error: expected statement before ')' token config.c:224:11: error: expected identifier before '(' token

Does changing line 220 of config.c to

    int c = (cf->fgetc)(cf);

Quoted 6 lines

Quoted 1 line

fix it?

I also had to change line 224 to the following

                c = (cf->fgetc)(cf);

Once both places were changes, it compiled successfully.

Sounds like a parser bug to me.