661962 - GCC 4.6 warning: "nsINIParser.cpp:91:14: warning: variable ‘rv’ set but not used"

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Daniel Holbert [:dholbert]]

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]

Attachment: Patch v1

I am getting intuition that NS_ENSURE_SUCCESS must be called for other platforms also.

Comment 2

[Benjamin Smedberg]

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]

Attachment: Patch v2

Comment 4

[Daniel Holbert [:dholbert]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/bc627f4c6daa

Comment 5

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/bc627f4c6daa