[PATCH libxtrans] Clean up some 'unused variable' warnings

Ian Romanick idr at freedesktop.org
Mon Mar 22 21:32:21 PDT 2010


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeff Smith wrote:
> 
> From: Ian Romanick <idr at freedesktop.org>
> To: Jeff Smith <whydoubt at yahoo.com>
> Cc: xorg-devel at lists.x.org
> Sent: Mon, March 22, 2010 4:38:08 PM
> Subject: Re: [PATCH libxtrans] Clean up some 'unused variable' warnings
> 
>> Specific comments below.  Given the nature of the changes and some of my
>> comments, I think these should be broken into a separate patch for each
>> variable.  If nothing else, it makes reviewing the changes easier.
> 
> I had figured given their close proximity, a single patch would be fine, but OK.

They start close, but I think I've read books shorter than that
function.  Given that the code is actually correct (see below), I won't
complain if this comes back as a single patch.

>> If tmpport is only assigned to once, make it const.
> 
> OK.
> 
>> I always get nervous about patches that move variable declarations in
>> the presence of heavily ifdef'ed code.  Whichever #if paths you don't
>> test are broken. :)  sockname is also used previously around lines 1534
>> and 1575 in the unpatched code.
> 
> I already checked for other instances of the variables.  For the locations you
> mention, "sockname" is actually just part of a text string.

Ah, you are correct.  I misread the code the first time I was reviewing it.

>> Both hostp and servp are used in a single if-block.  I think they should
>> be moved into those blocks and get the same const treatment (becoming
>> 'struct hostent *const hostp') that I recommended for tmpport.
> 
> OK.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuoRFMACgkQX1gOwKyEAw/+2gCcDOaK3YORAE85PGv30ROgv36r
vpoAmgIogctYfOxlE6UyzEff9tRQ7I56
=zFTK
-----END PGP SIGNATURE-----


More information about the xorg-devel mailing list