patch, 2 partscat-file: Fix an gcc -Wuninitialized warning

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.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

An alternative solution may look like this (note: *untested*):

    diff --git a/builtin/cat-file.c b/builtin/cat-file.c
    index ad29000..e50b20f 100644
    --- a/builtin/cat-file.c
    +++ b/builtin/cat-file.c
    @@ -193,7 +193,6 @@ static int batch_one_object(const char *obj_name, int print_contents)
     	unsigned char sha1[20];
     	enum object_type type = 0;
     	unsigned long size;
    -	void *contents;
     
     	if (!obj_name)
     	   return 1;
    @@ -204,16 +203,11 @@ static int batch_one_object(const char *obj_name, int print_contents)
     		return 0;
     	}
     
    -	if (print_contents == BATCH)
    -		contents = read_sha1_file(sha1, &type, &size);
    -	else
    -		type = sha1_object_info(sha1, &size);
    +	type = sha1_object_info(sha1, &size);
     
     	if (type <= 0) {
     		printf("%s missing\n", obj_name);
     		fflush(stdout);
    -		if (print_contents == BATCH)
    -			free(contents);
     		return 0;
     	}
     
    @@ -221,6 +215,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
     	fflush(stdout);
     
     	if (print_contents == BATCH) {
    +		void *contents = read_sha1_file(sha1, &type, &size);
     		write_or_die(1, contents, size);
     		printf("\n");
     		fflush(stdout);
    -- 

However, this would add an additional call to sha1_object_info() to the "--batch" code path, with potential performance consequences (again untested). Also, if you are paranoid, I guess you should check that the (type,size) returned by sha1_object_info() was the same as that returned by read_sha1_file(). ;-)

ATB, Ramsay Jones

 builtin/cat-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Tue, Mar 26, 2013 at 07:20:11PM +0000, 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".

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.

Yeah, I don't think that is worth it.

I note that we do not actually check that contents != NULL after calling read_sha1_file, either (nor that sha1_object_info does not return an error). I suspect cat-file could segfault under the right conditions.

-Peff

On Tue, Mar 26, 2013 at 03:35:39PM -0400, Jeff King wrote:

Oh nevermind, we do. We just do it by checking that "type" was filled in correctly, which can combine the check for both read_sha1_file and sha1_object_info. Sorry for the noise.

-Peff

Jeff King wrote:

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

(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

On Thu, Mar 28, 2013 at 06:48:43PM +0000, Ramsay Jones wrote:

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

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

Jeff King wrote:

Yep. :)