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
tool_doswin: Fix uninitialized field warning #3254
Conversation
Aren't empty initializers C++? I'm mainly a C++ developer, so I'm not 100 % sure, but I thought the universal zero initializer was |
On 9 Nov 2018, at 10:20, Marcel Raad ***@***.***> wrote:
Aren't empty initializers C++? I'm mainly a C++ developer, so I'm not 100 % sure, but I thought the universal zero initializer was {} for C++ and {0} for C.
You might be right, I don’t have a copy of the standard handy right now to check. clang is happy with either when compiling with `-std=c89` for whatever that means.
|
The partial struct initialization in 397664a caused a warning on uninitialized MODULEENTRY32 struct members: /src/tool_doswin.c:681:3: warning: missing initializer for field 'th32ModuleID' of 'MODULEENTRY32 {aka struct tagMODULEENTRY32}' [-Wmissing-field-initializers] This is sort of a bogus warning as the remaining members will be set to zero by the compiler, as all omitted members are. Nevertheless, remove the warning by omitting all members and setting the dwSize members explicitly. Closes #xxxx
d686fa0
to
6b6b653
Compare
Following up on this now that I had more time, |
silly appveyor-flaky test =( |
@MarcelRaad are you happy with this patch to go in? Would be nice to get back to a warning free windows cross-compilation. |
I'm not 100 % sure if changing the value to 0 makes the warning go away, I'd hope it does by now - old GCC versions still warned on this IIRC. And unfortunately, I don't have access to my development environment right now. But the change looks good to me. |
Unless there are objections I propose to push this later today to try and squash the warnings in the autobuilds. |
Just tried with https://godbolt.org/z/oeVJU-, seems to work in GCC 4.7 and later! In case it unexpectedly doesn't, using |
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.
The proposed change is fine and AFAIK still technically partial initialization, which is completely fine and defined behavior though they don't call it that in the standard. An object is either initialized or it's not. The standard basically says if there are less values than elements then the remaining are 0. struct foo bar = {0} is partial initialization but is so common it won't throw a warning as Marcel has shown. Any other value would probably cause it if extra warnings are enabled, which I didn't know when I sent it upstream since the CI passed.
the chosen answer of this question has a good breakdown: https://stackoverflow.com/questions/10828294/c-and-c-partial-initialization-of-automatic-structure |
Thanks for the reviews! The windows crosscompilation autobuilds should have two warnings less tomorrow. |
The partial struct initialization in 397664a caused a warning in the buildfarm on uninitialized
MODULEENTRY32
struct members:This is sort of a bogus warning as the remaining members will be set to zero by the compiler, as all omitted members are. Nevertheless, remove the warning by omitting all members and setting the
dwSize
member explicitly. (Settingth32ModuleID
to 1 as it's documented value is will only shift the warning to complain about the next field, so we need to do all or nothing.)@jay does this seem reasonable to you?