Skip to content

randomenv: drop self define of 'environ' #33675

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.

Sign up for GitHub

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

Jump to bottom
Closed
sipa wants to merge 1 commit into from
Closed

randomenv: drop self define of 'environ' #33675

sipa wants to merge 1 commit into from

Conversation

@sipa
Copy link
Member

@sipa sipa commented Oct 22, 2025

This variable/symbol/macro ought to be defined by the system includes, not us.

Alternative to #33570.

@sipa
randomenv: drop self define of 'environ'
d9283f4 
This variable/symbol/macro ought to be defined by the system includes, not us.
@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 22, 2025 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33675.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK hebasto, laanwj

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

hebasto
hebasto reviewed Oct 22, 2025
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.

@laanwj laanwj removed the Windows label Oct 22, 2025
@laanwj
Copy link
Member

laanwj commented Oct 22, 2025

Concept ACK

@maflcko
Copy link
Member

maflcko commented Oct 22, 2025

(mimicking drahtbot, bleeb blop)

https://github.com/bitcoin/bitcoin/actions/runs/18710961104/job/53359113198?pr=33675#step:9:1582:

[ 13%] Building CXX object src/util/CMakeFiles/bitcoin_util.dir/__/randomenv.cpp.o
cd /home/admin/actions-runner/_work/_temp/build/src/util && /bin/ccache /usr/bin/clang++ --target=arm64-apple-darwin -isysroot/home/admin/actions-runner/_work/_temp/depends/SDKs/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers -nostdlibinc -iwithsysroot/usr/include/c++/v1 -iwithsysroot/usr/include -iframeworkwithsysroot/System/Library/Frameworks --target=arm64-apple-darwin -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -I/home/admin/actions-runner/_work/_temp/build/src -I/home/admin/actions-runner/_work/_temp/src -mmacos-version-min=14.0 -mlinker-version=711 -O2 -O2 -g -std=c++20 -fPIC -fvisibility=hidden -fdebug-prefix-map=/home/admin/actions-runner/_work/_temp/src=. -fmacro-prefix-map=/home/admin/actions-runner/_work/_temp/src=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -mbranch-protection=bti -Werror -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -MD -MT src/util/CMakeFiles/bitcoin_util.dir/__/randomenv.cpp.o -MF CMakeFiles/bitcoin_util.dir/__/randomenv.cpp.o.d -o CMakeFiles/bitcoin_util.dir/__/randomenv.cpp.o -c /home/admin/actions-runner/_work/_temp/src/randomenv.cpp  
/home/admin/actions-runner/_work/_temp/src/randomenv.cpp:312:69: error: use of undeclared identifier 'environ'
  312 |     hasher << &hasher << &RandAddStaticEnv << &malloc << &errno << &environ;
      |                                                                     ^
/home/admin/actions-runner/_work/_temp/src/randomenv.cpp:452:9: error: use of undeclared identifier 'environ'
  452 |     if (environ) {
      |         ^
/home/admin/actions-runner/_work/_temp/src/randomenv.cpp:453:28: error: use of undeclared identifier 'environ'
  453 |         for (size_t i = 0; environ[i]; ++i) {
      |                            ^
/home/admin/actions-runner/_work/_temp/src/randomenv.cpp:454:48: error: use of undeclared identifier 'environ'; did you mean 'union'?
  454 |             hasher.Write((const unsigned char*)environ[i], strlen(environ[i]));
      |                                                ^~~~~~~
      |                                                union
/home/admin/actions-runner/_work/_temp/src/randomenv.cpp:454:48: error: expected expression
/home/admin/actions-runner/_work/_temp/src/randomenv.cpp:454:67: error: use of undeclared identifier 'environ'
  454 |             hasher.Write((const unsigned char*)environ[i], strlen(environ[i]));
      |                                                                   ^
6 errors generated.

@sipa sipa closed this Oct 22, 2025
@sipa
Copy link
Member Author

sipa commented Oct 22, 2025

It appears that mac OS actually requires defining it manually.

@fanquake fanquake mentioned this pull request Oct 27, 2025
Merged
@fanquake
Copy link
Member

fanquake commented Oct 27, 2025

Documented this in #33714.

hebasto added a commit that referenced this pull request Nov 4, 2025
@hebasto
Merge #33714: random: scope environ extern to macOS, BSDs and Illumos
96614ff 
79d6f45 random: scope environ extern to macOS, BSDs and Illumos (fanquake)

Pull request description:

  These platforms fail to compile with it removed.
  macOS: #33675
  BSDs / Illumos: hebasto/bitcoin-core-nightly#79.

ACKs for top commit:
  l0rinc:
    ACK 79d6f45
  hebasto:
    re-ACK 79d6f45.

Tree-SHA512: dcaa15f0939d65a804107ceb110037f44d0ff70759f4d42fcc497a9c173ac28b1287b867f01732224788d1c1f9c883565bafc3abed3ccf28f1b67f23997ce3cf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hebasto hebasto hebasto left review comments

Assignees

No one assigned

Labels

Utils/log/libs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sipa @DrahtBot @laanwj @maflcko @fanquake @hebasto