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...
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: mozilla9
Assigned To: Atul Aggarwal
: Nathan Froyd [:froydnj]
Depends on:
Blocks: buildwarning
  Show dependency treegraph
Reported: 2011-06-03 15:48 PDT by Daniel Holbert [:dholbert] (vacation, returning 2/27)
Modified: 2011-09-07 07:57 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 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);
104     fd = _wfopen(path.get(), READ_BINARYMODE);
105 #else
106     nsCAutoString path;
107     rv = aFile->GetNativePath(path);
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 User image 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 User image Benjamin Smedberg [: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 User image Atul Aggarwal 2011-08-31 14:43:07 PDT
Created attachment 557334 [details] [diff] [review]
Patch v2
Comment 4 User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 2011-09-06 14:47:51 PDT

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