{"thread":{"id":"10892","subject":"[PATCH] Fix Solaris compiler warnings","startedAt":"2007-11-15T22:19:11Z","lastAt":"2007-11-17T14:31:09Z","messageCount":10,"participants":["Guido Ostkamp","Alex Riesen","Junio C Hamano","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"60037","messageId":"Pine.LNX.4.64.0711152317140.7416@bianca.dialin.t-online.de","threadId":"10892","inReplyTo":null,"subject":"[PATCH] Fix Solaris compiler warnings","fromName":"Guido Ostkamp","fromEmail":"git@ostkamp.fastmail.fm","sentAt":"2007-11-15T22:19:11Z","receivedAt":"2007-11-15T22:19:11Z","isPatch":true,"sender":{"key":"git@ostkamp.fastmail.fm","avatar":null},"body":"Hello,\n\nthe below patch fixes some compiler warnings returned by Solaris Workshop \nCompilers.\n\n CC builtin-apply.o\n\"builtin-apply.c\", line 686: warning: statement not reached\n CC utf8.o\n\"utf8.c\", line 287: warning: statement not reached\n CC xdiff/xdiffi.o\n\"xdiff/xdiffi.c\", line 261: warning: statement not reached\n CC xdiff/xutils.o\n\"xdiff/xutils.c\", line 236: warning: statement not reached\n\nSigned-off-by: Guido Ostkamp \n---\n builtin-apply.c | 1 -\n utf8.c | 1 -\n xdiff/xdiffi.c | 2 --\n xdiff/xutils.c | 2 --\n 4 files changed, 0 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8edcc08..91f8752 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -683,7 +683,6 @@ static char *git_header_name(char *line, int llen)\n \t\t\t}\n \t\t}\n \t}\n-\treturn NULL;\n }\n\n /* Verify that we recognize the lines following a git header */\ndiff --git a/utf8.c b/utf8.c\nindex 8095a71..9efcdb9 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -284,7 +284,6 @@ int print_wrapped_text(const char *text, int indent, int indent2, int width)\n \t\t\ttext++;\n \t\t}\n \t}\n-\treturn w;\n }\n\n int is_encoding_utf8(const char *name)\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 5cb7171..1bad846 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -257,8 +257,6 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \t\t\treturn ec;\n \t\t}\n \t}\n-\n-\treturn -1;\n }\n\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 2ade97b..d7974d1 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -232,8 +232,6 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\treturn i1 >= s1 && i2 >= s2;\n \t} else\n \t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n-\n-\treturn 0;\n }\n\n static unsigned long xdl_hash_record_with_whitespace(char const **data,\n-- \n1.5.3.5.721.g039b\n"},{"id":"60042","messageId":"20071115230002.GA24069@steel.home","threadId":"10892","inReplyTo":"Pine.LNX.4.64.0711152317140.7416@bianca.dialin.t-online.de","subject":"Re: [PATCH] Fix Solaris compiler warnings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-15T23:00:02Z","receivedAt":"2007-11-15T23:00:02Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Guido Ostkamp, Thu, Nov 15, 2007 23:19:11 +0100:\n> Hello,\n>\n> the below patch fixes some compiler warnings returned by Solaris Workshop \n> Compilers.\n>\n> CC builtin-apply.o\n> \"builtin-apply.c\", line 686: warning: statement not reached\n> CC utf8.o\n> \"utf8.c\", line 287: warning: statement not reached\n> CC xdiff/xdiffi.o\n> \"xdiff/xdiffi.c\", line 261: warning: statement not reached\n\nAll these are wrong. That's a fantastically broken piece of compiler\n\n> CC xdiff/xutils.o\n> \"xdiff/xutils.c\", line 236: warning: statement not reached\n\nThis one is right. Accidentally, as it seems\n"},{"id":"60043","messageId":"7v1war3xrq.fsf@gitster.siamese.dyndns.org","threadId":"10892","inReplyTo":"20071115230002.GA24069@steel.home","subject":"Re: [PATCH] Fix Solaris compiler warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-15T23:16:25Z","receivedAt":"2007-11-15T23:16:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Guido Ostkamp, Thu, Nov 15, 2007 23:19:11 +0100:\n>> Hello,\n>>\n>> the below patch fixes some compiler warnings returned by Solaris Workshop \n>> Compilers.\n>>\n>> CC builtin-apply.o\n>> \"builtin-apply.c\", line 686: warning: statement not reached\n>> CC utf8.o\n>> \"utf8.c\", line 287: warning: statement not reached\n>> CC xdiff/xdiffi.o\n>> \"xdiff/xdiffi.c\", line 261: warning: statement not reached\n>\n> All these are wrong. That's a fantastically broken piece of compiler\n\nEh?\n\nI've looked at builtin-apply and utf8 cases but these returns\nare after an endless loop whose exit paths always return\ndirectly, so these return statements are in fact never reached.\n\nDumber compilers may not notice and if you remove these returns\nthey may start complaining, though. \n"},{"id":"60064","messageId":"20071116074850.GA3400@steel.home","threadId":"10892","inReplyTo":"7v1war3xrq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix Solaris compiler warnings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-16T07:48:50Z","receivedAt":"2007-11-16T07:48:50Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Fri, Nov 16, 2007 00:16:25 +0100:\n> Alex Riesen writes:\n> \n> > Guido Ostkamp, Thu, Nov 15, 2007 23:19:11 +0100:\n> >> Hello,\n> >>\n> >> the below patch fixes some compiler warnings returned by Solaris Workshop \n> >> Compilers.\n> >>\n> >> CC builtin-apply.o\n> >> \"builtin-apply.c\", line 686: warning: statement not reached\n> >> CC utf8.o\n> >> \"utf8.c\", line 287: warning: statement not reached\n> >> CC xdiff/xdiffi.o\n> >> \"xdiff/xdiffi.c\", line 261: warning: statement not reached\n> >\n> > All these are wrong. That's a fantastically broken piece of compiler\n> \n> Eh?\n> \n> I've looked at builtin-apply and utf8 cases but these returns\n> are after an endless loop whose exit paths always return\n> directly, so these return statements are in fact never reached.\n> \n> Dumber compilers may not notice and if you remove these returns\n> they may start complaining, though. \n\nHmm... Guido, I owe you an appology. Still, consider this patch\ninstead (it does not fix the return in xdiff/xdiffi.c though):\n\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8edcc08..6267396 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -668,13 +668,13 @@ static char *git_header_name(char *line, int llen)\n \t\tdefault:\n \t\t\tcontinue;\n \t\tcase '\\n':\n-\t\t\treturn NULL;\n+\t\t\tgoto eol;\n \t\tcase '\\t': case ' ':\n \t\t\tsecond = name+len;\n \t\t\tfor (;;) {\n \t\t\t\tchar c = *second++;\n \t\t\t\tif (c == '\\n')\n-\t\t\t\t\treturn NULL;\n+\t\t\t\t\tgoto eol;\n \t\t\t\tif (c == '/')\n \t\t\t\t\tbreak;\n \t\t\t}\n@@ -683,6 +683,7 @@ static char *git_header_name(char *line, int llen)\n \t\t\t}\n \t\t}\n \t}\n+eol:\n \treturn NULL;\n }\n \ndiff --git a/utf8.c b/utf8.c\nindex 8095a71..50c46af 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -262,7 +262,7 @@ int print_wrapped_text(const char *text, int indent, int indent2, int width)\n \t\t\t\t\tprint_spaces(indent);\n \t\t\t\tfwrite(start, text - start, 1, stdout);\n \t\t\t\tif (!c)\n-\t\t\t\t\treturn w;\n+\t\t\t\t\tbreak;\n \t\t\t\telse if (c == '\\t')\n \t\t\t\t\tw |= 0x07;\n \t\t\t\tspace = text;\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 2ade97b..533ff76 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -230,10 +230,9 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\t\ti2++;\n \t\t}\n \t\treturn i1 >= s1 && i2 >= s2;\n-\t} else\n-\t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n+\t}\n \n-\treturn 0;\n+\treturn s1 == s2 && !memcmp(l1, l2, s1);\n }\n \n static unsigned long xdl_hash_record_with_whitespace(char const **data,\n"},{"id":"60069","messageId":"7vve82zh5g.fsf@gitster.siamese.dyndns.org","threadId":"10892","inReplyTo":"20071116074850.GA3400@steel.home","subject":"Re: [PATCH] Fix Solaris compiler warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-16T09:14:19Z","receivedAt":"2007-11-16T09:14:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Junio C Hamano, Fri, Nov 16, 2007 00:16:25 +0100:\n>> Alex Riesen writes:\n>> \n>> > Guido Ostkamp, Thu, Nov 15, 2007 23:19:11 +0100:\n>> ...\n>> >> CC builtin-apply.o\n>> >> \"builtin-apply.c\", line 686: warning: statement not reached\n>> >> CC utf8.o\n>> >> \"utf8.c\", line 287: warning: statement not reached\n>> >> CC xdiff/xdiffi.o\n>> >> \"xdiff/xdiffi.c\", line 261: warning: statement not reached\n>> >\n>> > All these are wrong. That's a fantastically broken piece of compiler\n>> \n>> I've looked at builtin-apply and utf8 cases but these returns\n>> are after an endless loop whose exit paths always return\n>> directly, so these return statements are in fact never reached.\n>> ...\n>\n> Hmm... Guido, I owe you an appology. Still, consider this patch\n> instead (it does not fix the return in xdiff/xdiffi.c though):\n\nIf you are referring to the \"xdiff/xdiffi.c:line 261\" one (which\nI did not say if I looked at it or not), I think there is\nnothing to fix there, either. In front of itt is a big fat loop\ncontrolled with:\n\n\tfor (ec = 1;; ec++) {\n\t\t...\n\t}\n\nand only exits from there are returns. Two \"break\" appear but\nthey are breaking out of nested inner loops and would not escape\nthis outermost loop.\n"},{"id":"60115","messageId":"Pine.LNX.4.64.0711162346270.7281@bianca.dialin.t-online.de","threadId":"10892","inReplyTo":"20071116074850.GA3400@steel.home","subject":"Re: [PATCH] Fix Solaris compiler warnings","fromName":"Guido Ostkamp","fromEmail":"git@ostkamp.fastmail.fm","sentAt":"2007-11-16T22:52:01Z","receivedAt":"2007-11-16T22:52:01Z","isPatch":true,"sender":{"key":"git@ostkamp.fastmail.fm","avatar":null},"body":"On Fri, 16 Nov 2007, Alex Riesen wrote:\n> Hmm... Guido, I owe you an appology.\n\naccepted ;-)\n\n> Still, consider this patch instead (it does not fix the return in \n> xdiff/xdiffi.c though):\n\nIf your intention is to have a final 'return' statement for stupid \ncompilers, this should work (though I haven't verified it on a live system \nI think it looks reasonably well).\n\nAre you going to include it?\n\nWhat about the xdiff/xdiffi.c problem that should also be solved?\n\nRegards\n\nGuido\n"},{"id":"60128","messageId":"20071117094617.GD4086@steel.home","threadId":"10892","inReplyTo":"Pine.LNX.4.64.0711162346270.7281@bianca.dialin.t-online.de","subject":"[PATCH] Rewrite some function exit paths to avoid \"unreachable code\" traps","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-17T09:46:17Z","receivedAt":"2007-11-17T09:46:17Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Noticed by Guido Ostkamp for Sun's Workshop cc.\n\nOriginally-by: Guido Ostkamp \nSigned-off-by: Alex Riesen \n---\nGuido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:\n>\n> What about the xdiff/xdiffi.c problem that should also be solved?\n>\n\nHere you go.\n\n builtin-apply.c | 5 +++--\n utf8.c | 2 +-\n xdiff/xdiffi.c | 14 +++++++-------\n xdiff/xutils.c | 5 ++---\n 4 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8edcc08..6267396 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -668,13 +668,13 @@ static char *git_header_name(char *line, int llen)\n \t\tdefault:\n \t\t\tcontinue;\n \t\tcase '\\n':\n-\t\t\treturn NULL;\n+\t\t\tgoto eol;\n \t\tcase '\\t': case ' ':\n \t\t\tsecond = name+len;\n \t\t\tfor (;;) {\n \t\t\t\tchar c = *second++;\n \t\t\t\tif (c == '\\n')\n-\t\t\t\t\treturn NULL;\n+\t\t\t\t\tgoto eol;\n \t\t\t\tif (c == '/')\n \t\t\t\t\tbreak;\n \t\t\t}\n@@ -683,6 +683,7 @@ static char *git_header_name(char *line, int llen)\n \t\t\t}\n \t\t}\n \t}\n+eol:\n \treturn NULL;\n }\n \ndiff --git a/utf8.c b/utf8.c\nindex 8095a71..50c46af 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -262,7 +262,7 @@ int print_wrapped_text(const char *text, int indent, int indent2, int width)\n \t\t\t\t\tprint_spaces(indent);\n \t\t\t\tfwrite(start, text - start, 1, stdout);\n \t\t\t\tif (!c)\n-\t\t\t\t\treturn w;\n+\t\t\t\t\tbreak;\n \t\t\t\telse if (c == '\\t')\n \t\t\t\t\tw |= 0x07;\n \t\t\t\tspace = text;\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 5cb7171..365d768 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -110,7 +110,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \t\t\t\tspl->i1 = i1;\n \t\t\t\tspl->i2 = i2;\n \t\t\t\tspl->min_lo = spl->min_hi = 1;\n-\t\t\t\treturn ec;\n+\t\t\t\tgoto end;\n \t\t\t}\n \t\t}\n \n@@ -145,7 +145,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \t\t\t\tspl->i1 = i1;\n \t\t\t\tspl->i2 = i2;\n \t\t\t\tspl->min_lo = spl->min_hi = 1;\n-\t\t\t\treturn ec;\n+\t\t\t\tgoto end;\n \t\t\t}\n \t\t}\n \n@@ -184,7 +184,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \t\t\tif (best > 0) {\n \t\t\t\tspl->min_lo = 1;\n \t\t\t\tspl->min_hi = 0;\n-\t\t\t\treturn ec;\n+\t\t\t\tgoto end;\n \t\t\t}\n \n \t\t\tfor (best = 0, d = bmax; d >= bmin; d -= 2) {\n@@ -208,7 +208,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \t\t\tif (best > 0) {\n \t\t\t\tspl->min_lo = 0;\n \t\t\t\tspl->min_hi = 1;\n-\t\t\t\treturn ec;\n+\t\t\t\tgoto end;\n \t\t\t}\n \t\t}\n \n@@ -254,11 +254,11 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \t\t\t\tspl->min_lo = 0;\n \t\t\t\tspl->min_hi = 1;\n \t\t\t}\n-\t\t\treturn ec;\n+\t\t\tgoto end;\n \t\t}\n \t}\n-\n-\treturn -1;\n+end:\n+\treturn ec;\n }\n \n \ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 2ade97b..533ff76 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -230,10 +230,9 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\t\ti2++;\n \t\t}\n \t\treturn i1 >= s1 && i2 >= s2;\n-\t} else\n-\t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n+\t}\n \n-\treturn 0;\n+\treturn s1 == s2 && !memcmp(l1, l2, s1);\n }\n \n static unsigned long xdl_hash_record_with_whitespace(char const **data,\n-- \n1.5.3.5.750.g9f37\n"},{"id":"60130","messageId":"200711171139.32631.robin.rosenberg.lists@dewire.com","threadId":"10892","inReplyTo":"20071117094617.GD4086@steel.home","subject":"Re: [PATCH] Rewrite some function exit paths to avoid \"unreachable code\" traps","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-11-17T10:39:32Z","receivedAt":"2007-11-17T10:39:32Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"lördag 17 november 2007 skrev Alex Riesen:\n> Noticed by Guido Ostkamp for Sun's Workshop cc.\n> \n> Originally-by: Guido Ostkamp \n> Signed-off-by: Alex Riesen \n> ---\n> Guido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:\n> >\n> > What about the xdiff/xdiffi.c problem that should also be solved?\n> >\n> \n\nPlease... This just looks bad. I'm sure we'll have fixup patches on the list\nto fix those gotos. \n\nDo we support any such stupid compiler that requires a dummy goto?\nIf so we could just add a macro to compat-util.h\n\n#if stupid_compiler\n#define DUMMY_RETURN(x) return x;\n#else\n#define DUMMY_RETURN(x)\n#endif\n\nand then use it like this:\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8edcc08..91f8752 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -683,7 +683,6 @@ static char *git_header_name(char *line, int llen)\n                        }\n                }\n        }\n-       return NULL;\n+ DUMMY_RETURN(NULL)\n  }\n\nMy vote is for Guidos patch and fallback to the suggestion above if we support\nreally stupid compilers.\n\n-- robin\n"},{"id":"60132","messageId":"20071117122317.GA2716@steel.home","threadId":"10892","inReplyTo":"200711171139.32631.robin.rosenberg.lists@dewire.com","subject":"Re: [PATCH] Rewrite some function exit paths to avoid \"unreachable code\" traps","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-17T12:23:17Z","receivedAt":"2007-11-17T12:23:17Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Robin Rosenberg, Sat, Nov 17, 2007 11:39:32 +0100:\n> lördag 17 november 2007 skrev Alex Riesen:\n> > Noticed by Guido Ostkamp for Sun's Workshop cc.\n> > \n> > Originally-by: Guido Ostkamp \n> > Signed-off-by: Alex Riesen \n> > ---\n> > Guido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:\n> > >\n> > > What about the xdiff/xdiffi.c problem that should also be solved?\n> > >\n> > \n> \n> Please... This just looks bad. I'm sure we'll have fixup patches on the list\n> to fix those gotos. \n> \n> Do we support any such stupid compiler that requires a dummy goto?\n\nIt is more for the compilers we don't know about yet.\n\nUserspace programming, especially with intent to be portable, often\nmeans supporting *bugs* of the platform where it happens.\n"},{"id":"60144","messageId":"200711171531.09545.robin.rosenberg.lists@dewire.com","threadId":"10892","inReplyTo":"20071117122317.GA2716@steel.home","subject":"Re: [PATCH] Rewrite some function exit paths to avoid \"unreachable code\" traps","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-11-17T14:31:09Z","receivedAt":"2007-11-17T14:31:09Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"lördag 17 november 2007 skrev Alex Riesen:\n> Robin Rosenberg, Sat, Nov 17, 2007 11:39:32 +0100:\n> > lördag 17 november 2007 skrev Alex Riesen:\n> > > Noticed by Guido Ostkamp for Sun's Workshop cc.\n> > > \n> > > Originally-by: Guido Ostkamp \n> > > Signed-off-by: Alex Riesen \n> > > ---\n> > > Guido Ostkamp, Fri, Nov 16, 2007 23:52:01 +0100:\n> > > >\n> > > > What about the xdiff/xdiffi.c problem that should also be solved?\n> > > >\n> > > \n> > \n> > Please... This just looks bad. I'm sure we'll have fixup patches on the list\n> > to fix those gotos. \n> > \n> > Do we support any such stupid compiler that requires a dummy goto?\n> \n> It is more for the compilers we don't know about yet.\n> \n> Userspace programming, especially with intent to be portable, often\n> means supporting *bugs* of the platform where it happens.\n\n*If* it happens. We do not workaround every hypothetical compiler bug or every hypotetical\nbuggy compiler. Compilers we don't know about does not \"exist\", expect for the perfectly\nconfirming ones, but they aren't buggy.\n\nIt seems the return in utf8.c was introduced by mistake and Junio has made his decision now.\n\n-- robin\n"}]}