1307665 - Use MOZ_MUST_USE in netwerk/protocol/ftp

Status: RESOLVED WONTFIX

(Core :: Networking, defect)

Description

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Subtask of bug 1303701.

Comment 1

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b8a7ea9717888be4d4a30dce22b70aec96e79fe

Too many oranges.

Comment 2

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Failed tests:
dom/tests/mochitest/whatwg/test_postMessage_origin.xhtml
dom/xul/test/test_bug497875.xul
browser/base/content/test/urlbar/browser_canonizeURL.js

This line will try to remove a connection which does not exist:
https://dxr.mozilla.org/mozilla-central/rev/ea104eeb14cc54da9a06c3766da63f73117723a0/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#268

Is it intentional/safe here?

Comment 3

[Valentin Gosu [:valentin]]

Yes, that seems to be safe. If the call returns NS_ERROR_FAILURE, connection will be null, and we handle the case in the line below that. So you can just ignore the result value, or assert that it is FAILURE or NS_OK.

Comment 4

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Attachment: Bug 1307665 - Use MOZ_MUST_USE in netwerk/protocol/ftp

Review commit: https://reviewboard.mozilla.org/r/84118/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/84118/

Comment 5

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb33ac8e5539ecf0a4711a74a6890ca481766324

Comment 6

[Valentin Gosu [:valentin]]

Comment on attachment 8798699 [details]
Bug 1307665 - Use MOZ_MUST_USE in netwerk/protocol/ftp

https://reviewboard.mozilla.org/r/84118/#review83366

Please use MOZ_ASSERT, or Unused << rv;
I have a feeling some of these calls might fails from time to time, and even if it's nightly and aurora, I'd hate to have to track down crashes caused by non-fatal error codes. The code is barely used anyway.

Comment 7

[Patrick McManus [:mcmanus]]

I'm only taking security fixes to the ftp code these days.