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

RESOLVED FIXED in mozilla9

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dholbert, Assigned: Atul Aggarwal)

Tracking

(Blocks: 1 bug)

Trunk
mozilla9
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(1 attachment, 1 obsolete attachment)

1.05 KB, patch
bsmedberg
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
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?)

Updated

6 years ago
Blocks: 187528
(Assignee)

Comment 1

6 years ago
Created attachment 557021 [details] [diff] [review]
Patch v1

I am getting intuition that NS_ENSURE_SUCCESS must be called for other platforms also.
Assignee: nobody → atulagrwl
Status: NEW → ASSIGNED

Updated

6 years ago
Attachment #557021 - Flags: review?(benjamin)
Comment on attachment 557021 [details] [diff] [review]
Patch v1

GetNativePath cannot fail on non-Windows, so you can remove rv entirely.
Attachment #557021 - Flags: review?(benjamin) → review-
(Assignee)

Comment 3

6 years ago
Created attachment 557334 [details] [diff] [review]
Patch v2
Attachment #557021 - Attachment is obsolete: true
Attachment #557334 - Flags: review?(benjamin)
Attachment #557334 - Flags: review?(benjamin) → review+
(Reporter)

Comment 4

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/bc627f4c6daa
Whiteboard: [build_warning] → [build_warning][inbound]
(Reporter)

Updated

6 years ago
Target Milestone: --- → mozilla9
http://hg.mozilla.org/mozilla-central/rev/bc627f4c6daa
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [build_warning][inbound] → [build_warning]
You need to log in before you can comment on or make changes to this bug.