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

Ian Romanick idr at freedesktop.org
Mon Mar 22 14:38:08 PDT 2010


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

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.

Jeff Smith wrote:
> Signed-off-by: Jeff Smith <whydoubt at yahoo.com>
> ---
>  Xtranssock.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Xtranssock.c b/Xtranssock.c
> index 7106f5d..80659b3 100644
> --- a/Xtranssock.c
> +++ b/Xtranssock.c
> @@ -1431,19 +1431,14 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>      char		ntopbuf[INET6_ADDRSTRLEN];
>      int			resetonce = 0;
>  #endif
> -    struct sockaddr_in	sockname;
>  #ifdef XTHREADS_NEEDS_BYNAMEPARAMS
>      _Xgethostbynameparams hparams;
>      _Xgetservbynameparams sparams;
>  #endif
> -    struct hostent	*hostp;
> -    struct servent	*servp;
> -    unsigned long 	tmpaddr;
>  #ifdef X11_t
>      char	portbuf[PORTBUFSIZE];
>  #endif
>  
> -    long		tmpport;
>      char 		hostnamebuf[256];		/* tmp space */
>  
>      PRMSG (2,"SocketINETConnect(%d,%s,%s)\n", ciptr->fd, host, port);
> @@ -1467,7 +1462,7 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>  
>      if (is_numeric (port))
>      {
> -	tmpport = X_TCP_PORT + strtol (port, (char**)NULL, 10);
> +	long tmpport = X_TCP_PORT + strtol (port, (char**)NULL, 10);
>  	sprintf (portbuf, "%lu", tmpport);
>  	port = portbuf;

If tmpport is only assigned to once, make it const.

>      }
> @@ -1619,6 +1614,11 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>      }
>  #else
>      {
> +	struct sockaddr_in	sockname;

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.

> +	struct hostent	*hostp;
> +	struct servent	*servp;

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.

> +	unsigned long 	tmpaddr;
> +
>  	/*
>  	 * Build the socket name.
>  	 */
> @@ -1680,7 +1680,7 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>  	    }
>  	    sockname.sin_port = htons (servp->s_port);
>  	} else {
> -	    tmpport = strtol (port, (char**)NULL, 10);
> +	    long tmpport = strtol (port, (char**)NULL, 10);
>  	    if (tmpport < 1024 || tmpport > USHRT_MAX)
>  		return TRANS_CONNECT_FAILED;
>  	    sockname.sin_port = htons (((unsigned short) tmpport));

Same comment as above RE: const.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkun4z8ACgkQX1gOwKyEAw/w7gCeJ36aexH0tbyRJsPYOIWhnvfT
jjEAn1JQ4b79U82TF3WDe7FDO3aAcUhn
=Wx5y
-----END PGP SIGNATURE-----


More information about the xorg-devel mailing list