Last Comment Bug 661962 - GCC 4.6 warning: "nsINIParser.cpp:91:14: warning: variable ‘rv’ set but not used"
: GCC 4.6 warning: "nsINIParser.cpp:91:14: warning: variable ‘rv’ set but not u...
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla9
Assigned To: Atul Aggarwal
:
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2011-06-03 15:48 PDT by Daniel Holbert [:dholbert]
Modified: 2011-09-07 07:57 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (572 bytes, patch)
2011-08-30 15:37 PDT, Atul Aggarwal
benjamin: review-
Details | Diff | Splinter Review
Patch v2 (1.05 KB, patch)
2011-08-31 14:43 PDT, Atul Aggarwal
benjamin: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2011-06-03 15:48:13 PDT
Build warning when building with g++ 4.6:
{
xpcom/build/nsINIParser.cpp:91:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]
}

The flagged code is:

89 nsINIParser::Init(nsILocalFile* aFile)
90 {
91     nsresult rv;
[...]
99 #ifdef XP_WIN
100     nsAutoString path;
101     rv = aFile->GetPath(path);
102     NS_ENSURE_SUCCESS(rv, rv);
103 
104     fd = _wfopen(path.get(), READ_BINARYMODE);
105 #else
106     nsCAutoString path;
107     rv = aFile->GetNativePath(path);
108 
109     fd = fopen(path.get(), READ_BINARYMODE);
110 #endif

Note that in the XP_WIN case, we *do* check rv with NS_ENSURE_SUCCESS.

But in the "#else" case (e.g. on Linux), we don't check rv at all. (hence the warning)

Perhaps that case needs its own NS_ENSURE_SUCCESS(rv, rv)?  (Or alternately, perhaps we only need |rv| inside the XP_WIN ifdef?)
Comment 1 Atul Aggarwal 2011-08-30 15:37:19 PDT
Created attachment 557021 [details] [diff] [review]
Patch v1

I am getting intuition that NS_ENSURE_SUCCESS must be called for other platforms also.
Comment 2 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-08-31 08:13:45 PDT
Comment on attachment 557021 [details] [diff] [review]
Patch v1

GetNativePath cannot fail on non-Windows, so you can remove rv entirely.
Comment 3 Atul Aggarwal 2011-08-31 14:43:07 PDT
Created attachment 557334 [details] [diff] [review]
Patch v2
Comment 4 Daniel Holbert [:dholbert] 2011-09-06 14:47:51 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/bc627f4c6daa

Note You need to log in before you can comment on or make changes to this bug.