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

test: fix warning in test_environment.cc #36846

Conversation

RaisinTen
Copy link
Contributor

warning log:

../test/cctest/test_environment.cc: In constructor ‘RedirectStdErr::RedirectStdErr(const char*)’:
../test/cctest/test_environment.cc:77:12: warning: ignoring return value of ‘FILE* freopen(const char*, const char*, FILE*)’, declared with attribute warn_unused_result [-Wunused-result]
     freopen(filename_, "w", stderr);
     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 8, 2021
@RaisinTen RaisinTen force-pushed the test/fix-warning-in-test_environment.cc branch from f5706f3 to f67a8bd Compare January 9, 2021 09:02
@RaisinTen
Copy link
Contributor Author

cc @nodejs/testing

@targos
Copy link
Member

targos commented Jan 9, 2021

The USE macro doesn't work in this context? See

node/src/env.cc

Line 467 in a45a404

USE(script->Run(context()));
for example.

@RaisinTen
Copy link
Contributor Author

@targos thanks for mentioning that. I actually tried (void) <the-FILE*> and it still produced the warning. The ! seems to be necessary. I'm not quite sure how I'm supposed to include the header though given that none of the tests use USE. Do I hardcode deps/v8/src/base/macros.h in?

@targos
Copy link
Member

targos commented Jan 9, 2021

Sorry, it's not a macro for us. The function is defined in src/util.h. You can use it like this: node::USE(...)

@RaisinTen
Copy link
Contributor Author

@targos thanks, now the warning is gone. :)

warning log:
../test/cctest/test_environment.cc: In constructor ‘RedirectStdErr::RedirectStdErr(const char*)’:
../test/cctest/test_environment.cc:77:12: warning: ignoring return value of ‘FILE* freopen(const char*, const char*, FILE*)’, declared with attribute warn_unused_result [-Wunused-result]
     freopen(filename_, "w", stderr);
     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
@RaisinTen RaisinTen force-pushed the test/fix-warning-in-test_environment.cc branch from 9ae438f to 3e7ef4b Compare January 13, 2021 15:01
@RaisinTen RaisinTen added the review wanted PRs that need reviews. label Jan 17, 2021
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

RSLGTM

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 18, 2021
@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member

jasnell commented Jan 18, 2021

Landed in 341bbd3

@jasnell jasnell closed this Jan 18, 2021
jasnell pushed a commit that referenced this pull request Jan 18, 2021
```
warning log:
../test/cctest/test_environment.cc: In constructor   \
‘RedirectStdErr::RedirectStdErr(const char*)’:
../test/cctest/test_environment.cc:77:12: warning:   \
ignoring return value of ‘FILE* freopen(const char*, \
const char*, FILE*)’, declared with attribute
warn_unused_result [-Wunused-result]
     freopen(filename_, "w", stderr);
     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
```

PR-URL: #36846
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RaisinTen RaisinTen deleted the test/fix-warning-in-test_environment.cc branch January 19, 2021 13:49
@ruyadorno ruyadorno removed the review wanted PRs that need reviews. label Jan 21, 2021
ruyadorno pushed a commit that referenced this pull request Jan 22, 2021
```
warning log:
../test/cctest/test_environment.cc: In constructor   \
‘RedirectStdErr::RedirectStdErr(const char*)’:
../test/cctest/test_environment.cc:77:12: warning:   \
ignoring return value of ‘FILE* freopen(const char*, \
const char*, FILE*)’, declared with attribute
warn_unused_result [-Wunused-result]
     freopen(filename_, "w", stderr);
     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
```

PR-URL: #36846
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Jan 22, 2021
ruyadorno pushed a commit that referenced this pull request Jan 25, 2021
```
warning log:
../test/cctest/test_environment.cc: In constructor   \
‘RedirectStdErr::RedirectStdErr(const char*)’:
../test/cctest/test_environment.cc:77:12: warning:   \
ignoring return value of ‘FILE* freopen(const char*, \
const char*, FILE*)’, declared with attribute
warn_unused_result [-Wunused-result]
     freopen(filename_, "w", stderr);
     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
```

PR-URL: #36846
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request May 1, 2021
```
warning log:
../test/cctest/test_environment.cc: In constructor   \
‘RedirectStdErr::RedirectStdErr(const char*)’:
../test/cctest/test_environment.cc:77:12: warning:   \
ignoring return value of ‘FILE* freopen(const char*, \
const char*, FILE*)’, declared with attribute
warn_unused_result [-Wunused-result]
     freopen(filename_, "w", stderr);
     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
```

PR-URL: #36846
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants