FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix warning in test_environment.cc by RaisinTen · Pull Request #36846 · nodejs/node · GitHub

/ node Public

test: fix warning in test_environment.cc - #36846

Closed
RaisinTen wants to merge 1 commit into
nodejs:masterfrom
RaisinTen:test/fix-warning-in-test_environment.cc
ConversationCommits (1)ChecksFiles changed
Closed

test: fix warning in test_environment.cc#36846
RaisinTen wants to merge 1 commit into
nodejs:masterfrom
RaisinTen:test/fix-warning-in-test_environment.cc

Conversation

RaisinTen commented Jan 8, 2021

Copy link
Copy Markdown
Member

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 added the test Issues and PRs related to the tests. label Jan 8, 2021
RaisinTen force-pushed the test/fix-warning-in-test_environment.cc branch from f5706f3 to f67a8bd Compare January 9, 2021 09:02

RaisinTen commented Jan 9, 2021

Copy link
Copy Markdown
Member Author

cc @nodejs/testing

targos commented Jan 9, 2021

Copy link
Copy Markdown
Member

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 commented Jan 9, 2021

Copy link
Copy Markdown
Member 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 commented Jan 9, 2021

Copy link
Copy Markdown
Member

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 commented Jan 9, 2021

Copy link
Copy Markdown
Member Author

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

test: fix warning in test_environment.cc
3e7ef4b 
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 force-pushed the test/fix-warning-in-test_environment.cc branch from 9ae438f to 3e7ef4b Compare January 13, 2021 15:01
RaisinTen added the review wanted PRs that need reviews. label Jan 17, 2021
aduh95 approved these changes Jan 18, 2021
View reviewed changes

aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

aduh95 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 18, 2021

nodejs-github-bot commented Jan 18, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/35610/

jasnell commented Jan 18, 2021

Copy link
Copy Markdown
Member

Landed in 341bbd3

jasnell closed this Jan 18, 2021
jasnell pushed a commit that referenced this pull request Jan 18, 2021
test: fix warning in test_environment.cc
341bbd3 
```
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 deleted the test/fix-warning-in-test_environment.cc branch January 19, 2021 13:49
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
test: fix warning in test_environment.cc
3d20415 
```
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 mentioned this pull request Jan 22, 2021
Merged
ruyadorno pushed a commit that referenced this pull request Jan 25, 2021
test: fix warning in test_environment.cc
8d49ce9 
```
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
test: fix warning in test_environment.cc
90122d8 
```
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 mentioned this pull request May 3, 2021
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
aduh95 aduh95 approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL