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
bpo-45666: Use %S
for MSVC and %s
elsewhere for swprintf
in _testembed.c
#29341
Conversation
%S
-> %ls
in swprintf
in _testembed.c
%S
-> %ls
in swprintf
in _testembed.c
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.
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.
%ls
is not applicable here.
One possible solutions is to use %S
on Windows and %s
on Linux (it should be tested). Or maybe there is a portable way.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
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.
Following Serhiy's suggestion, maybe this is better:
You can also use |
Programs/_testembed.c
Outdated
const char *envvar = getenv("TESTFROZEN"); | ||
return check_use_frozen_modules(envvar); | ||
wchar_t *frozen = Py_DecodeLocale(envvar, NULL); |
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.
If you are going to use Py_DecodeLocale()
, you need to check its result for failure, and free the dynamic allocated memory after use. You should also add a special case for getenv()
returning NULL.
@Fidget-Spinner's suggestion is much simpler.
This reverts commit c67be03.
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Thank you @serhiy-storchaka and @Fidget-Spinner for your help! I am coming from Rust background and C is very hard for me! (no borrow checker 😬, you have to remember to deallocate memory!) But, I will do my best to get used to it 🙂 |
%S
-> %ls
in swprintf
in _testembed.c
%S
for MSVC and %s
elsewhere for swprintf
in _testembed.c
@sobolevn glad to help. Your comment also reminded me I should seriously try out Rust one day :). I still suck at it. |
@Fidget-Spinner you can take a look at |
Refs #29307
https://bugs.python.org/issue45666