-
-
Notifications
You must be signed in to change notification settings - Fork 706
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
fix some unsafe behavior in std.format #4427
Conversation
| * the difference between the starts of the arrays | ||
| */ | ||
| @trusted private pure nothrow @nogc | ||
| size_t arrayPtrDiff(const void[] array1, const void[] array2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this return ptrdiff_t or at least assert that array1.ptr >= array2.ptr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed
|
What is made safe by this and where is the @safe unittest that shows that. |
|
@burner Walter is fixing various safety related bugs. In this case, the next version of dmd will disallow accessing See also dlang/dmd#5860 |
|
@ZombineDev thanks. @WalterBright I would have liked this link to the dmd PR in the original PR description. Figuring out what the purpose of this PR is would have been easier this way. |
814af1e to
2be0355
Compare
|
LGTM |
| @@ -988,7 +988,7 @@ struct FormatSpec(Char) | |||
| const widthOrArgIndex = parse!uint(tmp); | |||
| enforceFmt(tmp.length, | |||
| text("Incorrect format specifier %", trailing[i .. $])); | |||
| i = tmp.ptr - trailing.ptr; | |||
| i = arrayPtrDiff(tmp, trailing); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could use a more general ptrValue safe function:
i = tmp.ptrValue - trailing.ptrValue;
See dlang/druntime#1590 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntrel see new PR I added to try and make a nice wrapper for this: dlang/druntime#1592
|
Auto-merge toggled on |
|
This pull request introduced a regression: |
- fix incorrect pointer diff computation introduced by PR dlang#4427 commit 2be0355
- fix incorrect pointer diff computation introduced by PR dlang#4427 commit 2be0355
No description provided.