950344 - Fix unused-but-set-variable 'client' warning in pkix_pl_httpdefaultclient.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Andrei V]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.66 Safari/537.36

Steps to reproduce:

I found a build warning in 521:13.14 pkix_pl_httpdefaultclient.c: In function ‘pkix_pl_HttpDefaultClient_KeepAliveSession’:
521:13.16 pkix_pl_httpdefaultclient.c:1135:36: warning: variable ‘client’ set but not used [-Wunused-but-set-variable] - Andrei V
521:13.16          PKIX_PL_HttpDefaultClient *client = NULL;
521:13.16                                     ^

I intend to fix it.

Comment 1

[Andrei V]

Attachment: first_patch

I moved line 1135 which was initialized with NULL to line 1147 and I initialized it with variable needed. This was my first bug fix.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 8347619 [details] [diff] [review]
first_patch

Review of attachment 8347619 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch. Please make the improvements below.

::: security/nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_httpdefaultclient.c
@@ +1131,5 @@
>          SEC_HTTP_SERVER_SESSION session,
>          PRPollDesc **pPollDesc,
>          void *plContext)
>  {
> +       

Please remove both of these blank lines. (And note trailing whitespace.)

@@ +1143,5 @@
>                  PKIX_HTTPDEFAULTCLIENT_TYPE,
>                  plContext),
>                  PKIX_SESSIONNOTANHTTPDEFAULTCLIENT);
> + 		
> +	PKIX_PL_HttpDefaultClient *client = (PKIX_PL_HttpDefaultClient *)session;

Please remove the two lines above. Since the client variable isn't used, we should just delete it.

Also, NSS is compiled in C89 mode, not C99+ mode. So, we must always declare variables at the start of a block. This code will not compile with Microsoft Visual Studio.

Comment 3

[:Cykesiopka]

Attachment: bug950344_rm-client-pkix_pl_httpdefaultclient.c_by-Andrei-Vaduva_v2.patch

This is Andrei's patch with Brian's comments addressed.

Comment 4

[:Cykesiopka]

On behalf of Andrei: Thanks for the reviews!

Comment 5

[Elio Maldonado]

https://hg.mozilla.org/projects/nss/rev/60c35f8c4e0f