Re: [PATCH 2/2] cat-file: Fix an gcc -Wuninitialized warning

2013-03-28 Thread Ramsay Jones
Jeff King wrote:
 On Tue, Mar 26, 2013 at 07:20:11PM +, Ramsay Jones wrote:
 
 After commit cbfd5e1c (drop some obsolete x = x compiler warning
 hacks, 21-03-2013) removed a gcc specific hack, older versions of
 gcc now issue an 'contents' might be used uninitialized warning.
 In order to suppress the warning, we simply initialize the variable
 to NULL in it's declaration.
 
 I'm OK with this, if it's the direction we want to go. But I thought the
 discussion kind of ended as we do not care about these warnings on
 ancient versions of gcc; those people should use -Wno-error=uninitialized.

Hmm, I don't recall any agreement or conclusions being reached.
I guess I missed that!

 What version of gcc are you using? If it is the most recent thing
 reasonably available on msysgit, then I am more sympathetic. But if it's
 just an antique version of gcc, I am less so.

(see previous email for compiler versions).

I suppose it depends on what you consider antique. [I recently
downloaded the first C compiler from github. Yes, that is an
antique compiler! ;-)] I would call some of the compilers I use
a bit mature. :-P

Hmm, so are you saying that this patch is not acceptable because
I used a compiler that is no longer supported?

ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] cat-file: Fix an gcc -Wuninitialized warning

2013-03-28 Thread Jeff King
On Thu, Mar 28, 2013 at 06:48:43PM +, Ramsay Jones wrote:

  I'm OK with this, if it's the direction we want to go. But I thought the
  discussion kind of ended as we do not care about these warnings on
  ancient versions of gcc; those people should use -Wno-error=uninitialized.
 
 Hmm, I don't recall any agreement or conclusions being reached.
 I guess I missed that!

I think Jonathan said that and nobody disagreed, and I took it as a
conclusion.

 Hmm, so are you saying that this patch is not acceptable because
 I used a compiler that is no longer supported?

No, I just think we should come to a decision on how unreadable to make
the code in order to suppress incorrect warnings on old compilers. I can
see the point in either of the following arguments:

  1. These compilers are old, and we do not need to cater to them in the
 code because people can just _not_ set -Werror=uninitialized (or
 its equivalent). It is still worth catering to bugs in modern
 compilers that most devs use, because being able to set -Werror is
 helpful.

  2. The code is not made significantly less readable, especially if you
 put in a comment, so why not help these compilers.

When we can make the code more readable _and_ help the compiler, I think
it is a no-brainer. I am on the fence otherwise and don't care that
much. I just think we should apply the rule consistently.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] cat-file: Fix an gcc -Wuninitialized warning

2013-03-28 Thread Jonathan Nieder
Jeff King wrote:

 When we can make the code more readable _and_ help the compiler, I think
 it is a no-brainer.

Yep. :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html