# [PATCH] Clean up struct initalizations - Thread: https://externals.io/message/33450 - Original archive: https://news-web.php.net/php.internals/33450 - Started by Andy Lester on 2007-11-27T07:47:57+00:00 - Messages: 18 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #33450 — Andy Lester — 2007-11-27T07:47:57+00:00 URL: https://externals.io/message/33450#33450 I've been working on making PHP build under more stringent error checking, specifically -Wextra under GCC. This patch cleans up dozens of struct initializations that are valid C, but could hide future problems. xoxo, Andy -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance --- ## #33451 — Stanislav Malyshev — 2007-11-27T08:53:12+00:00 URL: https://externals.io/message/33450#33451 · In reply to: #33450 > I've been working on making PHP build under more stringent error > checking, specifically -Wextra under GCC. This patch cleans up dozens > of struct initializations that are valid C, but could hide future > problems. Doesn't C standard mandate filling up uninitialized fields with 0? -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #33452 — Marcus Boerger — 2007-11-27T10:31:43+00:00 URL: https://externals.io/message/33450#33452 · In reply to: #33451 Hello Stanislav, a global struct is filled up with NULL/0's while a local struct is not initialized at all. The patch simply provides more correct code and even adds a few consts. I think we should apply this to 5.3 and HEAD. Meaning we need it for both versions. marcus Tuesday, November 27, 2007, 9:53:12 AM, you wrote: > > […] > Doesn't C standard mandate filling up uninitialized fields with 0? > -- > Stanislav Malyshev, Zend Software Architect > stas@zend.com http://www.zend.com/ > (408)253-8829 MSN: stas@zend.com Best regards, Marcus --- ## #33454 — Andy Lester — 2007-11-27T13:49:42+00:00 URL: https://externals.io/message/33450#33454 · In reply to: #33451 > > […] > > Doesn't C standard mandate filling up uninitialized fields with 0? Yes. My patch is for human benefit, not the compiler's. -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance --- ## #33455 — Ilia Alshanetsky — 2007-11-27T14:00:05+00:00 URL: https://externals.io/message/33450#33455 · In reply to: #33454 IMHO {0} is sufficiently clear for human consumption. > > > > > […] > > > Yes. My patch is for human benefit, not the compiler's. > > -- > Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance > > -- > > > Ilia Alshanetsky --- ## #33456 — Alexey Zakhlestin — 2007-11-27T14:34:46+00:00 URL: https://externals.io/message/33450#33456 · In reply to: #33455 > IMHO {0} is sufficiently clear for human consumption. {NULL, 0, 0} brings real structure dimensions to the context, which is good (less need to check "headers"). > > > > […] > > Ilia Alshanetsky > > -- > > > > -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ --- ## #33457 — Andy Lester — 2007-11-27T15:12:41+00:00 URL: https://externals.io/message/33450#33457 · In reply to: #33456 > > > […] > > {NULL, 0, 0} brings real structure dimensions to the context, which is > good (less need to check "headers"). It also means that you don't get compiler errors any more like you would with {0}. Now, you can know that if there's a warning about the number of initializers that it's something you should pay attention to. The key for long-term maintenance is to clean up the slop so that the compiler doesn't squawk about it at high levels of warnings. Then when you do have real squawk-worthy problems, they don't get lost in the noise of meaningless warnings. -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance --- ## #33458 — Ilia Alshanetsky — 2007-11-27T15:49:04+00:00 URL: https://externals.io/message/33450#33458 · In reply to: #33457 GCC will spue a slew of warnings that can be safely ignored and in some cases are bogus. The pedantic changes only make the code harder to read and yield very little if any benefits in return. For larger, more complex structs like zvals it'll only create a meaningless mess. There is absolutely no reason that any half decent compiler will not be able to understand {0} for any declared struct. > > > > > […] > > It also means that you don't get compiler errors any more like you > would with {0}. Now, you can know that if there's a warning about > the number of initializers that it's something you should pay > attention to. > > The key for long-term maintenance is to clean up the slop so that > the compiler doesn't squawk about it at high levels of warnings. > Then when you do have real squawk-worthy problems, they don't get > lost in the noise of meaningless warnings. > > > -- > Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance > > > > Ilia Alshanetsky --- ## #33459 — Andy Lester — 2007-11-27T15:50:49+00:00 URL: https://externals.io/message/33450#33459 · In reply to: #33458 > GCC will spue a slew of warnings that can be safely ignored and in > some cases are bogus. The pedantic changes only make the code harder > to read and yield very little if any benefits in return. For larger, > more complex structs like zvals it'll only create a meaningless > mess. There is absolutely no reason that any half decent compiler > will not be able to understand {0} for any declared struct. My goal here is to improve long-term maintainability of the code, by letting compilers and tools like splint do their automated magic where possible. If this is not useful to the PHP core team, then that's fine, I'll walk away. Let me know either way. xoa -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance --- ## #33460 — Ilia Alshanetsky — 2007-11-27T15:57:32+00:00 URL: https://externals.io/message/33450#33460 · In reply to: #33459 Andy, There is absolutely nothing wrong with trying to make the code more maintainable, in fact I think everyone, myself included welcome any work in that direction. But at the same time we need to keep in mind that compiler is not always right and often will generate meaningless warnings even when there is absolutely nothing wrong the code. Changes for the sake of making the compiler happy with -Winsane-warnings ;-) does not really help maintainability. In the case of {0} vs {NULL, 0,0}, my opinion is that the former is far more readable and understandable. > > > > > […] > > > My goal here is to improve long-term maintainability of the code, by > letting compilers and tools like splint do their automated magic > where possible. If this is not useful to the PHP core team, then > that's fine, I'll walk away. Let me know either way. > > xoa > > -- > Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance > > > > Ilia Alshanetsky --- ## #33461 — Andy Lester — 2007-11-27T16:12:02+00:00 URL: https://externals.io/message/33450#33461 · In reply to: #33460 > There is absolutely nothing wrong with trying to make the code more > maintainable, in fact I think everyone, myself included welcome any > work in that direction. But at the same time we need to keep in mind > that compiler is not always right and often will generate > meaningless warnings even when there is absolutely nothing wrong the > code. Changes for the sake of making the compiler happy with - > Winsane-warnings ;-) does not really help maintainability. In the > case of {0} vs {NULL,0,0}, my opinion is that the former is far more > readable and understandable. I'm trying to look at the code with the eyes of a human, trying to use the compiler as my ally, not something to be circumvented. GCC has most of the features of a good lint, if we take advantage of it. Think of it as compiler-enforced coding standards. My goal has been to try to raise up the coding level of the code that I see, that I'm using in my day-to-day work. The more seat belts the better, and initializing structs is just one of them. Adding consts and fixing signed/unsigned mismatches are on my list, too, but there's just so much that's slop that I'm trying to get the low-hanging fruit early. xoa -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance --- ## #33463 — Stanislav Malyshev — 2007-11-27T18:23:05+00:00 URL: https://externals.io/message/33450#33463 · In reply to: #33454 > > […] > > > Yes. My patch is for human benefit, not the compiler's. Not sure I see how it helps - suppose we add a field into structure, does it mean one has to go over all the code that ever mentioned this structure and add zeroes? How helpful is that? -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #33464 — Andy Lester — 2007-11-27T18:25:45+00:00 URL: https://externals.io/message/33450#33464 · In reply to: #33463 > Not sure I see how it helps - suppose we add a field into structure, > does it mean one has to go over all the code that ever mentioned this > structure and add zeroes? How helpful is that? Makes sure you got them all, doesn't it? xoa -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance --- ## #33465 — Stanislav Malyshev — 2007-11-27T18:26:25+00:00 URL: https://externals.io/message/33450#33465 · In reply to: #33459 > My goal here is to improve long-term maintainability of the code, by I don't see how unwrapping all structures in initializers improves maintainability. If anything, it actually makes code harder to maintain - now on any change you need to go over all the code and fix all the initializers - a ton of completely meaningless work since a compiler can do it for you and the standard specifically provides means to do it. Now if there were non-zero initializers, that would be different, but with zero inits I don't see any reason not to leave them be. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #33466 — Alexey Zakhlestin — 2007-11-27T18:27:26+00:00 URL: https://externals.io/message/33450#33466 you are quoting c99… and we are still using "c90" mode, as far as I remember > > […] > > That's not what I read in C standard - it says that if struct is > partially initialized, the rest is zeroed out: > > Quoting http://c0x.coding-guidelines.com/6.7.8.html: > > 1671 If there are fewer initializers in a brace-enclosed list than there > are elements or members of an aggregate, or fewer characters in a string > literal used to initialize an array of known size than there are > elements in the array, the remainder of the aggregate shall be > initialized implicitly the same as objects that have static storage > duration. > > 1642 If an object that has static storage duration is not initialized > explicitly, then: > 1643 — if it has pointer type, it is initialized to a null pointer; > 1644 — if it has arithmetic type, it is initialized to (positive or > unsigned) zero; > 1645 — if it is an aggregate, every member is initialized (recursively) > according to these rules; > 1646 — if it is a union, the first named member is initialized > (recursively) according to these rule > > All compilers I know do behave this way. Do not confuse partial > initialization wiht no initialization at all, of course. > -- > Stanislav Malyshev, Zend Software Architect > stas@zend.com http://www.zend.com/ > (408)253-8829 MSN: stas@zend.com > > -- > > > > -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ --- ## #33467 — Stanislav Malyshev — 2007-11-27T18:30:35+00:00 URL: https://externals.io/message/33450#33467 · In reply to: #33464 > > […] > > Makes sure you got them all, doesn't it? No, the compiler makes sure I got them all, and it does it with {0}. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #33469 — Stanislav Malyshev — 2007-11-27T18:57:29+00:00 URL: https://externals.io/message/33450#33469 · In reply to: #33466 > you are quoting c99… > and we are still using "c90" mode, as far as I remember I couldn't find any c90 available online for free but anyway that's long-standing practice that all compilers I know follow. gcc's own manual says: -Wmissing-field-initializers Warn if a structure's initializer has some fields missing. For example, the following code would cause such a warning, because x.h is implicitly zero: struct s { int f, g, h; }; struct s x = { 3, 4 }; -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #33470 — Andy Lester — 2007-11-27T19:15:22+00:00 URL: https://externals.io/message/33450#33470 · In reply to: #33467 > No, the compiler makes sure I got them all, and it does it with {0}. I can tell where this is going. I'll back away now. xoa -- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance