bpo-29248: Fix readlink bug os#5577
Conversation
Signed-off-by: SSE4 <tomskside@gmail.com>
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
Signed-off-by: SSE4 <tomskside@gmail.com>
| and os.path.exists(r'C:\ProgramData'), | ||
| 'Test directories not found') | ||
| def test_29248(self): | ||
| target = os.readlink(r'C:\Users\All Users') |
There was a problem hiding this comment.
I would add the explanation from the tracker issue (why we're using this link instead of creating a fresh one to test) here as a comment.
|
@SSE4 please let us know when you sign the CLA form so we can merge this PR. |
|
@berkerpeksag I already did earlier today |
| @@ -0,0 +1 @@ | |||
| fix os.readlink() on Windows | |||
There was a problem hiding this comment.
Please make the news entry more descriptive and add "Patch by Craig Holmquist and Your Name."
| } | ||
| print_name = rdb->SymbolicLinkReparseBuffer.PathBuffer + | ||
| rdb->SymbolicLinkReparseBuffer.PrintNameOffset; | ||
| (rdb->SymbolicLinkReparseBuffer.PrintNameOffset / 2); |
There was a problem hiding this comment.
Can PrintNameOffset be odd?
There was a problem hiding this comment.
PathBuffer is of type wchar_t. Strictly this should use sizeof(wchar_t) instead of 2. Alternatively it can use two casts to have the compiler do the work: (wchar_t *)((char *)rdb->SymbolicLinkReparseBuffer.PathBuffer + rdb->SymbolicLinkReparseBuffer.PrintNameOffset).
| (rdb->SymbolicLinkReparseBuffer.PrintNameOffset / 2); | ||
|
|
||
| result = PyUnicode_FromWideChar(print_name, | ||
| rdb->SymbolicLinkReparseBuffer.PrintNameLength/2); |
There was a problem hiding this comment.
Strictly this should also use sizeof(wchar_t) instead of hard coding 2.
berkerpeksag
left a comment
There was a problem hiding this comment.
All review comments have been addressed and this looks good to me. I will wait for a day or two to give everyone a chance to look at the patch and merge it. Thanks!
|
@berkerpeksag okay, 3 days passed already, what's the next action to be done? |
|
Thanks @SSE4 for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
|
GH-5640 is a backport of this pull request to the 3.6 branch. |
The PrintNameOffset field of the reparse data buffer was treated as a number of characters instead of bytes. (cherry picked from commit 3c34aad) Co-authored-by: SSE4 <tomskside@gmail.com>
|
@Mariatta do I need to backport this to 3.7 branch manually or is there some sort of queue? |
|
Thanks @SSE4 for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
GH-5644 is a backport of this pull request to the 3.7 branch. |
The PrintNameOffset field of the reparse data buffer was treated as a number of characters instead of bytes. (cherry picked from commit 3c34aad) Co-authored-by: SSE4 <tomskside@gmail.com>
|
@berkerpeksag I think this was merged at the same time miss-islington was being deployed, so it missed that webhook event. I've reapplied the label to trigger the backport. |
|
@Mariatta thanks! |
|
@berkerpeksag I noticed that the news file is the only one in the repo to use CRLF. |
see https://bugs.python.org/issue29248
a year already has passed since patch was provided. what should be done to make this into python release?
/cc @gvanrossum @berkerpeksag @asvetlov
https://bugs.python.org/issue29248