Skip to content

Fixed warning when compiling pzstd with CPPFLAGS=-Wunused-result and …#4134

Merged
terrelln merged 2 commits into
facebook:devfrom
inventor500:fix-Wunused-result-warning
Sep 25, 2024
Merged

Fixed warning when compiling pzstd with CPPFLAGS=-Wunused-result and …#4134
terrelln merged 2 commits into
facebook:devfrom
inventor500:fix-Wunused-result-warning

Conversation

@inventor500

Copy link
Copy Markdown
Contributor

This should close #4133.
This problem is not particularly important, since this was a build warning and does not seem to effect anything, but it does remove a warning when compiling on GCC.

@neheb

neheb commented Sep 23, 2024

Copy link
Copy Markdown
Contributor

I don't think this is right. The code seems to be wanting to do https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom

Meaning

--- a/contrib/pzstd/Options.cpp
+++ b/contrib/pzstd/Options.cpp
@@ -322,7 +322,7 @@ Options::Status Options::parse(int argc, const char **argv) {
   g_utilDisplayLevel = verbosity;
   // Remove local input files that are symbolic links
   if (!followLinks) {
-      std::remove_if(localInputFiles.begin(), localInputFiles.end(),
+      localInputFiles.erase(std::remove_if(localInputFiles.begin(), localInputFiles.end(),
                      [&](const char *path) {
                         bool isLink = UTIL_isLink(path);
                         if (isLink && verbosity >= 2) {
@@ -332,7 +332,7 @@ Options::Status Options::parse(int argc, const char **argv) {
                                     path);
                         }
                         return isLink;
-                    });
+                    }), localInputFiles.end());
   }
 
   // Translate input files/directories into files to (de)compress

is the proper solution.

It doesn't affect anything since this is contrib and not really used.

edit: as the wiki page states, this should be using std::erase_if and not std::remove_if. But that needs C++20.

@Cyan4973
Cyan4973 requested a review from terrelln September 24, 2024 00:36

@terrelln terrelln left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Please make the change that @neheb is suggesting.

@terrelln

Copy link
Copy Markdown
Contributor

Thanks for the bug fix @inventor500 & @neheb!

@terrelln
terrelln merged commit 9215de5 into facebook:dev Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build Warning with -Wunused-return on gcc

5 participants