Skip to content

Conversation

al45tair
Copy link
Contributor

Pretty sure this can't ever trigger because of operating system limits on the size of environment strings, but since overflow tests are cheap, there seems no reason not to add one.

rdar://160307933

Pretty sure this can't ever trigger because of operating system
limits on the size of environment strings, but since overflow
tests are cheap, there seems no reason not to add one.

rdar://160307933
@al45tair al45tair requested a review from mikeash as a code owner September 17, 2025 14:12
@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

size_t totalLen = nameLen + 1 + valueLen + 1;
size_t totalLen;

if (__builtin_add_overflow(nameLen + 1, valueLen + 1, &totalLen)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a bit silly, but should we also overflow check the +2 (+1+1)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe we need to do that. I don't think std::strlen() returning SIZE_MAX is something we ever need worry about.

@al45tair
Copy link
Contributor Author

Linux had a failure in SwiftPM's tests, unrelated to this PR.

@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test Linux platform

1 similar comment
@dschaefer2
Copy link
Member

@swift-ci Please smoke test Linux platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants