You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "pitrou (via GitHub)" <gi...@apache.org> on 2026/02/04 16:00:46 UTC
[PR] GH-49146: [C++] Add option to disable atfork handlers [arrow]
pitrou opened a new pull request, #49148:
URL: https://github.com/apache/arrow/pull/49148
### Rationale for this change
### What changes are included in this PR?
### Are these changes tested?
### Are there any user-facing changes?
**This PR includes breaking changes to public APIs.** (If there are any breaking changes to public APIs, please explain which changes are breaking. If not, you can remove this.)
**This PR contains a "Critical Fix".** (If the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld), please provide explanation. If not, you can remove this.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49146: [C++] Add option to disable atfork handlers [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49148:
URL: https://github.com/apache/arrow/pull/49148#issuecomment-3922696779
After merging your PR, Conbench analyzed the 2 benchmarking runs that have been run so far on merge-commit ebaaf07adbd302e95e393b5b77d78c1c97ea3b70.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/64052964395) has more details. It also includes information about 12 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49146: [C++] Add option to disable atfork handlers [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49148:
URL: https://github.com/apache/arrow/pull/49148#issuecomment-3937931969
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit ebaaf07adbd302e95e393b5b77d78c1c97ea3b70.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/64366819125) has more details. It also includes information about 13 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49146: [C++] Add option to disable atfork handlers [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #49148:
URL: https://github.com/apache/arrow/pull/49148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49146: [C++] Add option to disable atfork handlers [arrow]
Posted by "adamreeve (via GitHub)" <gi...@apache.org>.
adamreeve commented on code in PR #49148:
URL: https://github.com/apache/arrow/pull/49148#discussion_r2779812348
##########
cpp/src/arrow/util/io_util.cc:
##########
@@ -1762,38 +1763,54 @@ Result<std::string> GetEnvVar(std::string_view name) {
#ifdef _WIN32
// On Windows, getenv() reads an early copy of the process' environment
// which doesn't get updated when SetEnvironmentVariable() is called.
- constexpr int32_t bufsize = 2000;
- char c_str[bufsize];
- auto res = GetEnvironmentVariableA(name.data(), c_str, bufsize);
- if (res >= bufsize) {
- return Status::CapacityError("environment variable value too long");
- } else if (res == 0) {
- return Status::KeyError("environment variable '", name, "'undefined");
- }
- return std::string(c_str);
+ std::string value(100, '\0');
+
+ uint32_t res = GetEnvironmentVariableA(name.data(), value.data(),
+ static_cast<uint32_t>(value.size()));
+ if (res >= value.size()) {
Review Comment:
This `>=` comparison was a bit confusing to me at first but this is fine as the `==` case should never happen.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org