Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake, common: silence cmake and gcc warnings #21484

Merged
merged 4 commits into from Apr 20, 2018

Conversation

tchaikov
Copy link
Contributor

No description provided.

@tchaikov tchaikov force-pushed the wip-cmake-common-cleanup branch 3 times, most recently from 7acb257 to 7cefddd Compare April 18, 2018 04:34
@tchaikov tchaikov force-pushed the wip-cmake-common-cleanup branch 2 times, most recently from 23e4d50 to 76aa00d Compare April 18, 2018 05:58
Preforker.h:111:8: warning: ignoring return value of ‘ssize_t
safe_write(int, const void*, size_t)’, declared with attribute
warn_unused_result [-Wunused-result]
        (void)safe_write(fd[1], &r, sizeof(r));
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and

compat.cc:36:28: warning: comparison between signed and unsigned integer
expressions [-Wsign-compare]
     if (off + sizeof(data) > len)
         ~~~~~~~~~~~~~~~~~~~^~~~~

Fixes: http://tracker.ceph.com/issues/23774
Signed-off-by: Kefu Chai <kchai@redhat.com>
it's intended behavior to use imported or alias target for dependencies
with name with "::" in it.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov tchaikov force-pushed the wip-cmake-common-cleanup branch 2 times, most recently from db3b69f to 43d251a Compare April 18, 2018 09:22
@tchaikov tchaikov requested a review from jdurgin April 19, 2018 03:28
@tchaikov
Copy link
Contributor Author

tchaikov commented Apr 19, 2018

@jdurgin can we have this PR in mimic?

@tchaikov
Copy link
Contributor Author

@@ -108,7 +108,7 @@ class Preforker {
int signal_exit(int r) {
if (forked) {
/* If we get an error here, it's too late to do anything reasonable about it. */
(void)safe_write(fd[1], &r, sizeof(r));
[[maybe_unused]] auto n = safe_write(fd[1], &r, sizeof(r));
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really a fan of what appears to be gcc/clang magic to do this but I leave it up to your judgement @tchaikov. Is there something wrong with what's in #21481?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@batrick there is nothing wrong with #21481. and IMHO, this is not gcc/clang magic, it's an attribute specifier defined in C++17. i think, it's just more explicit and more readable than the good old (void) way.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I'll close #21481 in favor of this.

the boost's download page offers the SHA256 hash, so it'd be easier to
verify the hash this way.

Signed-off-by: Kefu Chai <kchai@redhat.com>
to silence the warnings like

CMake Warning at CMakeLists.txt:73 (find_package):
  By not providing "Findgflags.cmake" in CMAKE_MODULE_PATH this project
has
  asked CMake to find a package configuration file provided by "gflags",
but
  CMake did not find one.

  Could not find a package configuration file provided by "gflags" with
any
  of the following names:

    gflagsConfig.cmake
    gflags-config.cmake

  Add the installation prefix of "gflags" to CMAKE_PREFIX_PATH or set
  "gflags_DIR" to a directory containing one of the above files.  If
"gflags"
  provides a separate development package or SDK, be sure it has been
  installed.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

i am dropping the commit bumping boost version to 1.67 out of this PR, so it is safer to be merged into mimic.

@tchaikov tchaikov merged commit 82cb956 into ceph:master Apr 20, 2018
@tchaikov tchaikov deleted the wip-cmake-common-cleanup branch April 20, 2018 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants