1737585 - NS_SUCCEEDED(rv); => value computed is not used [-Werror=unused-value]

Status: NEW

(Core :: DOM: Events, task)

Description

[Sylvestre Ledru [:Sylvestre]]

Found by gcc 12:
https://searchfox.org/mozilla-central/rev/ff5309e67b20a9976ea762f9ec5f657da0801490/dom/events/DataTransfer.cpp#654
https://searchfox.org/mozilla-central/rev/ff5309e67b20a9976ea762f9ec5f657da0801490/dom/events/DataTransfer.cpp#637
https://searchfox.org/mozilla-central/rev/ff5309e67b20a9976ea762f9ec5f657da0801490/dom/events/DataTransfer.cpp#654

/root/firefox-gcc-last/dom/events/DataTransfer.cpp: In static member function 'static void mozilla::dom::DataTransfer::GetExternalClipboardFormats(const int32_t&, const bool&, nsTArray<nsTString<char> >*)':
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsError.h:33:34: error: value computed is not used [-Werror=unused-value]
   33 | #define NS_SUCCEEDED(_nsresult) ((bool)MOZ_LIKELY(!NS_FAILED_impl(_nsresult)))
      |                                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/firefox-gcc-last/dom/events/DataTransfer.cpp:637:5: note: in expansion of macro 'NS_SUCCEEDED'
  637 |     NS_SUCCEEDED(rv);
      |     ^~~~~~~~~~~~
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsError.h:33:34: error: value computed is not used [-Werror=unused-value]
   33 | #define NS_SUCCEEDED(_nsresult) ((bool)MOZ_LIKELY(!NS_FAILED_impl(_nsresult)))
      |                                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/firefox-gcc-last/dom/events/DataTransfer.cpp:654:5: note: in expansion of macro 'NS_SUCCEEDED'
  654 |     NS_SUCCEEDED(rv);
      |     ^~~~~~~~~~~~

Comment 1

[ISHIKAWA, Chiaki]

I have tried to compile C-C TB using gcc-12 suite of compilers.

I think the above problems can be fixed by modifying each of them to use

NS_ENSURE_SUCCESS_VOID(rv);

Actually, g++-12 found a similar inappropriate use of NS_SUCCEEDED in dom/ipc/contentParent.cpp.
https://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#6639

We may need a bit of cleanup of the source code by fixing the problems found by gcc-12.

Comment 2

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, Chiaki from comment #1)

I have tried to compile C-C TB using gcc-12 suite of compilers.

I think the above problems can be fixed by modifying each of them to use

NS_ENSURE_SUCCESS_VOID(rv);

Of course, I am assuming the intention is to return immediately when an error occurs.

Actually, g++-12 found a similar inappropriate use of NS_SUCCEEDED in dom/ipc/contentParent.cpp.
https://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#6639
The code above cannot be fixed by NS_ENSURE_SUCCESS_VOID().

// Send the PageLoadPing after every 30 page loads, or on startup.
  if (++sPageLoadEventCounter >= 30) {
    NS_SUCCEEDED(NS_DispatchToMainThreadQueue(
        NS_NewRunnableFunction(
            "PageLoadPingIdleTask",
            [] { mozilla::glean_pings::Pageload.Submit("threshold"_ns); }),
        EventQueuePriority::Idle));
    sPageLoadEventCounter = 0;
  }

I simply changed it to print a warning when the argument to NS_SUCCEEDED returns failure code.
But someone needs to look into this.

We may need a bit of cleanup of the source code by fixing the problems found by gcc-12.