Closed
Bug 666989
Opened 13 years ago
Closed 13 years ago
Shmem.cpp:357:8: warning: variable ‘checkMappingFront’ set but not used [-Wunused-but-set-variable]
Categories
(Core :: IPC, defect)
Tracking
()
VERIFIED
FIXED
mozilla7
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
(Whiteboard: [build_warning][inbound])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
cjones
:
review+
|
Details | Diff | Splinter Review |
Filing bug on this GCC 4.6 warning:
ipc/glue/Shmem.cpp: In member function ‘void mozilla::ipc::Shmem::AssertInvariants() const’:
ipc/glue/Shmem.cpp:357:8: warning: variable ‘checkMappingFront’ set but not used [-Wunused-but-set-variable]
The flagged code is:
349 void
350 Shmem::AssertInvariants() const
351 {
352 NS_ABORT_IF_FALSE(mSegment, "NULL segment");
353 NS_ABORT_IF_FALSE(mData, "NULL data pointer");
354 NS_ABORT_IF_FALSE(mSize > 0, "invalid size");
355 // if the segment isn't owned by the current process, these will
356 // trigger SIGSEGV
357 char checkMappingFront = *reinterpret_cast<char*>(mData);
358 char checkMappingBack = *(reinterpret_cast<char*>(mData) + mSize - 1);
359 checkMappingFront = checkMappingBack; // avoid "unused" warnings
360 }
It looks like we're just assigning these checkMappingXXX vars to see if that triggers a segfault.
Line 359 is a hackaround for "unused" warnings in older GCC versions, but sadly, GCC 4.6 sees through it. :)
Perhaps we want to use a #pragma to selectively disable this warning for this block of code?
Or alternately, we could add:
checkMappingBack = checkMappingFront;
at the end of this function, so that both variables are both written & read, being fully "used".
Assignee | ||
Comment 1•13 years ago
|
||
...or we can cast the variables to (void), which IIRC is the best way to "use" unused variables.
Attachment #541734 -
Flags: review?(jones.chris.g)
Comment on attachment 541734 [details] [diff] [review]
fix v1: (void)
Please use |unused| for this (I love saying that!). E.g. |unused << checkMappingFront;|.
Assignee | ||
Comment 3•13 years ago
|
||
Nice -- I didn't realize we had that!
Assignee: nobody → dholbert
Attachment #541734 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #541778 -
Flags: review?(jones.chris.g)
Attachment #541734 -
Flags: review?(jones.chris.g)
Comment on attachment 541778 [details] [diff] [review]
fix v2: unused <<
Looking back on this code, I see that |mData| actually should have been declared |volatile|, but that's a bigger patch and separate problem.
Attachment #541778 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 5•13 years ago
|
||
Whiteboard: [build_warning][inbound]
Comment 6•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Comment 7•13 years ago
|
||
Verified that file ipc/glue/Shmem.cpp was updated in mozilla-central repository.
Is this enough to set the satus to VERIFIED-FIXED or there are other tests that need to be run?
Thanks!
Assignee | ||
Comment 8•13 years ago
|
||
Technically, you could do a Firefox build and verify that the warning's gone (or find the most recent linux "clobber" build on tinderbox and check its build log for the warning), but that's probably more work than a bug like this is worth.
So, I'd say that Comment 7 is sufficient in my book.
Comment 9•13 years ago
|
||
Changing status to VERIFIED-FIXED based on comment 8
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•