Closed Bug 1404035 Opened 7 years ago Closed 7 years ago

Address unused-result warnings exposed by MinGW build

Categories

(Core :: General, enhancement)

Product:
Core
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

Details

Attachments

(5 files)

Bug 1404035 Address unused result warnings in dom/ipc/
59 bytes, text/x-review-board-request
smaug
: review+
Details
Bug 1404035 Address unused-result warning in toolkit/components/filewatcher/NativeFileWatcherWin.cpp
59 bytes, text/x-review-board-request
smaug
: review+
Details
Bug 1404035 Address an unused-result warning in widget/windows/KeyboardLayout.cpp
59 bytes, text/x-review-board-request
jimm
: review+
Details
Bug 1404035 Address an unused result warning in gfx/thebes/DeviceManagerDx.cpp
59 bytes, text/x-review-board-request
Details
Bug 1404035 Address an unused result warning in netwerk/socket/nsNamedPipeIOLayer.cpp
59 bytes, text/x-review-board-request
bagder
: review+
u408661
: review+
Details 
We decorate some methods with warn_unused_result to indicate that the result should be used. In some places we don't use that result. Sometimes this a bug, sometimes it's on purpose, but we should address the warnings.
Try run showing the warning is gone: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fe0951c992944ad8507b751cb4c1d3604f2689d&selectedJob=133895138 (ignore the build breaks)

Try run covering platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=06646239c245c994ac2b01db9f9edc72aac6352a

Asking for review from various module owners.  The important thing to note is that while I may say "We can't do anything to handle the error" (or a similar assertive sounding statement) I am absolutely making a guess (sometimes educated, sometimes not).
Attachment #8913332 - Flags: review?(bugs)
Attachment #8913333 - Flags: review?(bugs)
Attachment #8913334 - Flags: review?(jmathies)
Attachment #8913335 - Flags: review?(jmuizelaar)
Attachment #8913336 - Flags: review?(hurley)
Comment on attachment 8913332 [details]
Bug 1404035 Address unused result warnings in dom/ipc/

https://reviewboard.mozilla.org/r/184692/#review189934
Attachment #8913332 - Flags: review?(bugs) → review+
Comment on attachment 8913333 [details]
Bug 1404035 Address unused-result warning in toolkit/components/filewatcher/NativeFileWatcherWin.cpp

https://reviewboard.mozilla.org/r/184694/#review189936
Attachment #8913333 - Flags: review?(bugs) → review+
Attachment #8913336 - Flags: review?(hurley) → review?(mcmanus)
I'm not comfortable enough with the named pipe code to ensure this is reasonable, hence the redirect to mcmanus...
Attachment #8913335 - Flags: review?(jmuizelaar) → review?(dvander)
Comment on attachment 8913335 [details]
Bug 1404035 Address an unused result warning in gfx/thebes/DeviceManagerDx.cpp

https://reviewboard.mozilla.org/r/184698/#review190298
Attachment #8913335 - Flags: review?(dvander) → review+
Comment on attachment 8913334 [details]
Bug 1404035 Address an unused-result warning in widget/windows/KeyboardLayout.cpp

https://reviewboard.mozilla.org/r/184696/#review190332

can we just use something like NS_WARNING?
Attachment #8913334 - Flags: review?(jmathies) → review+
Comment on attachment 8913334 [details]
Bug 1404035 Address an unused-result warning in widget/windows/KeyboardLayout.cpp

https://reviewboard.mozilla.org/r/184696/#review190332

For the one, yes. Updated.
Attachment #8913335 - Flags: review?(jmuizelaar)
Status: NEW → ASSIGNED
Comment on attachment 8913336 [details]
Bug 1404035 Address an unused result warning in netwerk/socket/nsNamedPipeIOLayer.cpp

https://reviewboard.mozilla.org/r/184700/#review193188

::: netwerk/socket/nsNamedPipeIOLayer.cpp:301
(Diff revision 2)
>  NamedPipeInfo::Disconnect()
>  {
>    MOZ_ASSERT(OnSocketThread(), "not on socket thread");
>  
>    nsresult rv = mNamedPipeService->RemoveDataObserver(mPipe, this);
> -  NS_WARN_IF(NS_FAILED(rv));
> +  if(NS_WARN_IF(NS_FAILED(rv))) {

Well, in case of failure returned from RemoveDataObserver(), should it really not to the rest of the cleanups?

Shouldn't the rest of the cleanups be done and _then_ the error returned?
Attachment #8913336 - Flags: review?(mcmanus)
(In reply to Daniel Stenberg [:bagder] from comment #18)
> Well, in case of failure returned from RemoveDataObserver(), should it
> really not to the rest of the cleanups?
> 
> Shouldn't the rest of the cleanups be done and _then_ the error returned?

Ah yes, true.
(Review flag was not populated, using ni)
Flags: needinfo?(daniel)
Comment on attachment 8913336 [details]
Bug 1404035 Address an unused result warning in netwerk/socket/nsNamedPipeIOLayer.cpp

https://reviewboard.mozilla.org/r/184700/#review194386
Attachment #8913336 - Flags: review+
Flags: needinfo?(daniel)
mozreview doesn't show one of the r+'s but it's there. Try run green.
Keywords: checkin-needed
This doesn't meet the review requirements in MozReview for Autoland to push it.
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/autoland.html#landing-commits
Flags: needinfo?(tom)
Keywords: checkin-needed
Comment on attachment 8913336 [details]
Bug 1404035 Address an unused result warning in netwerk/socket/nsNamedPipeIOLayer.cpp

https://reviewboard.mozilla.org/r/184700/#review195630

r+ing since Daniel's didn't take well enough for autoland...
Attachment #8913336 - Flags: review+
Pushed by hurley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2dd18979613d
Address unused result warnings in dom/ipc/ r=smaug
https://hg.mozilla.org/integration/autoland/rev/dc032a9e7456
Address unused-result warning in toolkit/components/filewatcher/NativeFileWatcherWin.cpp r=smaug
https://hg.mozilla.org/integration/autoland/rev/7763b3f3da5b
Address an unused-result warning in widget/windows/KeyboardLayout.cpp r=jimm
https://hg.mozilla.org/integration/autoland/rev/8effd11745d8
Address an unused result warning in gfx/thebes/DeviceManagerDx.cpp r=dvander
https://hg.mozilla.org/integration/autoland/rev/92a0d2f17a62
Address an unused result warning in netwerk/socket/nsNamedPipeIOLayer.cpp r=bagder,nwgh
Flags: needinfo?(tom)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: