[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