817176 - nsSmartCardMonitor.cpp:90:25: warning: unused variable ‘current’ [-Wunused-variable]

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Daniel Holbert [:dholbert]]

Build warning:
{
security/manager/ssl/src/nsSmartCardMonitor.cpp:90:25: warning: unused variable ‘current’ [-Wunused-variable]
}

from the top change in this cset:
https://hg.mozilla.org/mozilla-central/diff/9a4531d2d243/security/manager/ssl/src/nsSmartCardMonitor.cpp

The code in question is:

> 87 nsresult
> 88 SmartCardThreadList::Add(SmartCardMonitoringThread *thread)
> 89 {
> 90   SmartCardThreadEntry *current = new SmartCardThreadEntry(thread, head, nullptr,
> 91                                                            &head);
> 92   // OK to forget current here, it's on the list
> 93   return thread->Start();
> 94 }
https://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsSmartCardMonitor.cpp#87

Given the code-comment there, it looks like we're *intentionally* not using "current", and it's not a problem.

We can tell the compiler this is OK by adding
 (void)current;

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

This patch also fixes a whitespace style issue (add spaces around "=") and a typo (s/passwd/passed/) in contextual code.

Comment 2

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

Comment on attachment 687310 [details] [diff] [review]
fix

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

::: security/manager/ssl/src/nsSmartCardMonitor.cpp
@@ +73,5 @@
>  void
>  SmartCardThreadList::Remove(SECMODModule *aModule)
>  {
>    SmartCardThreadEntry *current;
> +  for (current = head; current; current = current->next) {

Nit: could fold the declaration into the for loop head too.

@@ +90,5 @@
>    SmartCardThreadEntry *current = new SmartCardThreadEntry(thread, head, nullptr,
>                                                             &head);
> +  // OK to forget current here, it's on the list.
> +  // (Use (void) to silence GCC "unused variable" warning.)
> +  (void)current;

How about mozilla::unused <<current?

Comment 3

[Daniel Holbert [:dholbert]]

nice, I forgot about the mozilla::unused construct.  Thanks for reminding me -- I'll use that. (and tweak the "for" loop as you suggest, too)

Comment 4

[Daniel Holbert [:dholbert]]

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/e0f8cc72dd06

Also: while adding the #include for unused.h at the top of the file, I noticed that we had an unnecessary "#include <assert.h>" a little lower, separated from all the other #includes. I removed it, because it's not needed or used.  (It actually was unused even back when it was added, in this CVS commit: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/security/manager/ssl/src&command=DIFF_FRAMESET&file=nsSmartCardMonitor.cpp&rev2=1.2&rev1=1.1 )

Comment 5

[Daniel Holbert [:dholbert]]

oops, my commit message ended up being slightly incorrect:
>  Bug 817176: Add "(void)" cast to silence GCC unused-var warning for an intentionally-unused variable. r=bsmith
... since I ended up using "unused <<" instead of (void).

It gets the point across, though, and I don't think it's worth a backout & re-landing, so I'll just leave it.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e0f8cc72dd06