Skip to content

Conversation

inventor500
Copy link
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
Copy link
Contributor

neheb commented Sep 23, 2024

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
Copy link
Contributor

@terrelln terrelln left a comment

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
Contributor

Thanks for the bug fix @inventor500 & @neheb!

@terrelln terrelln merged commit 9215de5 into facebook:dev Sep 25, 2024
91 checks passed
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