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

fix travis error with init time in mockenv #2418

Closed

Conversation

maysamyabandeh
Copy link
Contributor

/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));

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

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.

@facebook-github-bot
Copy link
Contributor

@maysamyabandeh updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

@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));
Copy link
Contributor

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.

@facebook-github-bot
Copy link
Contributor

@maysamyabandeh updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@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);
Copy link
Contributor

@sagar0 sagar0 Jun 6, 2017

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.

Copy link
Contributor Author

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!

Copy link
Contributor

@sagar0 sagar0 Jun 6, 2017

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.

Copy link
Contributor

@siying siying left a 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.

@facebook-github-bot
Copy link
Contributor

@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

yiwu-arbug pushed a commit that referenced this pull request Jun 9, 2017
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
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.

None yet

4 participants