-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix travis error with init time in mockenv #2418
Conversation
env/mock_env.cc
Outdated
@@ -379,7 +379,8 @@ class TestMemLogger : public Logger { | |||
gettimeofday(&now_tv, nullptr); | |||
const time_t seconds = now_tv.tv_sec; | |||
struct tm t; | |||
localtime_r(&seconds, &t); | |||
auto ret = localtime_r(&seconds, &t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to add "attribute((unused))" after ret, otherwise release build may complain ret is not used.
@maysamyabandeh updated the pull request - view changes |
@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
env/mock_env.cc
Outdated
@@ -379,7 +379,7 @@ class TestMemLogger : public Logger { | |||
gettimeofday(&now_tv, nullptr); | |||
const time_t seconds = now_tv.tv_sec; | |||
struct tm t; | |||
localtime_r(&seconds, &t); | |||
assert(localtime_r(&seconds, &t)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whatever inside assert will be optimized out in release build.
@maysamyabandeh updated the pull request - view changes - changes since last import |
c3a0185
to
4a21a68
Compare
@maysamyabandeh updated the pull request - view changes - changes since last import |
@@ -379,7 +379,8 @@ class TestMemLogger : public Logger { | |||
gettimeofday(&now_tv, nullptr); | |||
const time_t seconds = now_tv.tv_sec; | |||
struct tm t; | |||
localtime_r(&seconds, &t); | |||
auto ret __attribute__((__unused__)) = localtime_r(&seconds, &t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be __attribute__((unused)) to pass gcc and clang.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but attribute((unused)) is all over the code base!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting, didn't know (TIL)! Seems like we have both the versions in our code base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure it works for both of (release and debug ) * (clang and gcc) before commit.
@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: /home/travis/build/facebook/rocksdb/env/mock_env.cc: In member function ‘virtual void rocksdb::{anonymous}::TestMemLogger::Logv(const char*, va_list)’: /home/travis/build/facebook/rocksdb/env/mock_env.cc:391:53: error: ‘t.tm::tm_year’ may be used uninitialized in this function [-Werror=maybe-uninitialized] static_cast<int>(now_tv.tv_usec)); Closes #2418 Differential Revision: D5193597 Pulled By: maysamyabandeh fbshipit-source-id: 8801a3ef27f33eb419d534f7de747702cdf504a0
/home/travis/build/facebook/rocksdb/env/mock_env.cc: In member function ‘virtual void rocksdb::{anonymous}::TestMemLogger::Logv(const char*, va_list)’:
/home/travis/build/facebook/rocksdb/env/mock_env.cc:391:53: error: ‘t.tm::tm_year’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
static_cast(now_tv.tv_usec));