Re: svn commit: r295768 - head/usr.sbin/iostat
On 22 Feb 2016, at 10:56, David Chisnall wrote: > > On 19 Feb 2016, at 23:23, Dimitry Andric wrote: >> >> This warning is only produced when you use -Wall -W, and then initialize >> structs partially, i.e. you initialize some fields but not others. I >> think this is a quite reasonable warning for a high warning level. > > The warning is annoying in many ways. You ought to be able to zero > initialise any struct with {0}, but clang objects if you do this and requires > every field to be filled in. This warning really shouldn’t be enabled with > -Wall, because it has too hight a false positive rate. It isn't, it is in -W (a.k.a -Wextra). But gcc also warns in this case. E.g. if I use this example: struct foo { int i; int j; } bar[] = { { 42 }, { 43 } }; I get the following warnings from gcc -Wextra: $ gcc -Wextra -c initializers.c initializers.c:5:2: warning: missing initializer for field 'j' of 'struct foo' [-Wmissing-field-initializers] { 42 }, ^ initializers.c:3:6: note: 'j' declared here int j; ^ initializers.c:6:2: warning: missing initializer for field 'j' of 'struct foo' [-Wmissing-field-initializers] { 43 } ^ initializers.c:3:6: note: 'j' declared here int j; ^ Note that the warnings disappear if C99 initializers are used. -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 04:50:40PM -0800, Conrad Meyer wrote: C> > It is not quite as simple as this would make it sound. The elements or C> > members of an aggregate (e.g.) structure type are initialized as if it were C> > an object of static storage duration (i.e., to zero) if the initializer list C> > does not contain enough initializers for all members of the aggregate type, C> > per item 21 of section 6.7.8 of n1256.pdf. However, such initialization C> > does not necessarily need to zero any padding bytes that are present, which C> > may take unspecified values. Personally, I think this particular clang C> > warning can be too aggressive, especially for complex structs, but on the C> > other hand given the indeterminateness of padding, bzero/memset are often a C> > better choice anyway. C> C> By definition, padding byte contents are unused. There is no reason C> their values matter one way or another, so why do we care about the C> distinction between bzero and member zero initialization? Well, one can run bcmp() or a hashing function over entire structure, in that case contents of padding matter. But I still don't want to see this warning in -Wall, since it kills very useful C99 initializers. The code, that runs bcmp() or hashes over structs, should take care, and we shouldn't pessimize the entire build for its sake. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On 22 Feb 2016, at 10:15, Kubilay Kocak wrote: > > For the lay persons among us (I'm genuinely interested), what are the > the downsides to requiring initialization of all fields? Explicit initialisation, or initialisation in general? Being able to initialise the entire structure with code that will always initialise everything with zero makes the code less fragile because you can add a field to the structure and not have to find all of the places where it’s initialised. The clang warning makes it easy to find the places you need to change, but now you have code churn for no good reason. Implicit initialisation is often useful for generating more efficient code. If you’re intialising with a bunch of common fields, the compiler will end up creating a static variable and doing a memcpy (which the back end may optimise), which is very cheap. A load of code in libc ends up like this. > And in addition, the upsides, if any, of 'deferred' field initialization? The same as the upsides for deferred variable initialization. Modern compilers are good at tracking intraprocedural data flow. If you don’t initialise a field, then the compiler can typically tell that you’ve read from a field but not written to it. It’s generally good defensive programming for zero to have a well-defined meaning (C codifies this for pointers, Go codifies it for all structure types), but if you haven’t done this then the zero may be a valid but incorrect value and no amount of static analysis can tell you that the programmer did something that is valid but wrong. Some languages with richer type systems explicitly encode the invalidity of the variable in its type and transform it to the valid version once it is initialised (some also provide maybe types as a programmer-level constructs. You can implement them fairly trivially in C++, see for example LLVM’s ErrorOr<> template). Sane coding style for C (not style(9)) typically includes the principle of minimum scope, where variables must be declared as close to possible their initialisation. This helps minimise these problems, because you either declare the variable where it is initialised, or (if initialisation is conditional) right next to it where you can clearly see that it has been initialised on all code paths. Unfortunately, as you can see from the PVS results, style(9) could have been carefully designed to maximise the introduction of accidental bugs. David ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On 22/02/2016 8:56 PM, David Chisnall wrote: > On 19 Feb 2016, at 23:23, Dimitry Andric wrote: >> >> This warning is only produced when you use -Wall -W, and then >> initialize structs partially, i.e. you initialize some fields but >> not others. I think this is a quite reasonable warning for a high >> warning level. > > The warning is annoying in many ways. You ought to be able to zero > initialise any struct with {0}, but clang objects if you do this and > requires every field to be filled in. This warning really shouldn’t > be enabled with -Wall, because it has too hight a false positive > rate. For the lay persons among us (I'm genuinely interested), what are the the downsides to requiring initialization of all fields? And in addition, the upsides, if any, of 'deferred' field initialization? Is there a proper term for the above? > With regard to Bruce’s comment about padding, this is a known issue > in C11. There is an open DR about it and it’s scheduled for > discussion at the WG14 meeting in London in April. > > David > Koobs ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On 19 Feb 2016, at 23:23, Dimitry Andric wrote: > > This warning is only produced when you use -Wall -W, and then initialize > structs partially, i.e. you initialize some fields but not others. I > think this is a quite reasonable warning for a high warning level. The warning is annoying in many ways. You ought to be able to zero initialise any struct with {0}, but clang objects if you do this and requires every field to be filled in. This warning really shouldn’t be enabled with -Wall, because it has too hight a false positive rate. With regard to Bruce’s comment about padding, this is a known issue in C11. There is an open DR about it and it’s scheduled for discussion at the WG14 meeting in London in April. David ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Sun, Feb 21, 2016 at 3:00 PM, Andrew Turner wrote: > On Thu, 18 Feb 2016 20:08:01 + (UTC) > Alan Somers wrote: > >> Author: asomers >> Date: Thu Feb 18 20:08:01 2016 >> New Revision: 295768 >> URL: https://svnweb.freebsd.org/changeset/base/295768 >> >> Log: >> Fix compiler warnings in iostat >> >> Raise WARNS from 1 to 6 (the default) > > This breaks building with gcc: > I'll fix it in the morning. Thanks for reporting it. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Thu, 18 Feb 2016 20:08:01 + (UTC) Alan Somers wrote: > Author: asomers > Date: Thu Feb 18 20:08:01 2016 > New Revision: 295768 > URL: https://svnweb.freebsd.org/changeset/base/295768 > > Log: > Fix compiler warnings in iostat > > Raise WARNS from 1 to 6 (the default) This breaks building with gcc: ===> usr.sbin/ifmcstat (all) ===> usr.sbin/iostat (all) cc1: warnings being treated as errors /scratch/tmp/andrew/head-git/usr.sbin/iostat/iostat.c: In function 'devstats': /scratch/tmp/andrew/head-git/usr.sbin/iostat/iostat.c:800: warning: declaration of 'devname' shadows a global declaration /scratch/tmp/andrew/obj/powerpc.powerpc/scratch/tmp/andrew/head-git/tmp/usr/include/stdlib.h:282: warning: shadowed declaration is here /scratch/tmp/andrew/head-git/usr.sbin/iostat/iostat.c: In function 'cpustats': /scratch/tmp/andrew/head-git/usr.sbin/iostat/iostat.c:982: warning: declaration of 'time' shadows a global declaration /scratch/tmp/andrew/obj/powerpc.powerpc/scratch/tmp/andrew/head-git/tmp/usr/include/time.h:154: warning: shadowed declaration is here *** Error code 1 Stop. make[4]: stopped in /scratch/tmp/andrew/head-git/usr.sbin/iostat *** Error code 1 Stop. make[3]: stopped in /scratch/tmp/andrew/head-git/usr.sbin *** Error code 1 Stop. make[2]: stopped in /scratch/tmp/andrew/head-git *** Error code 1 Stop. make[1]: stopped in /scratch/tmp/andrew/head-git *** Error code 1 Stop. make: stopped in /scratch/tmp/andrew/head-git -Wshadow is added to the warning flags with WARNS >= 4. Andrew ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 6:50 PM, Conrad Meyer wrote: > On Fri, Feb 19, 2016 at 3:14 PM, Benjamin Kaduk wrote: > > On Fri, Feb 19, 2016 at 5:06 PM, Gleb Smirnoff > wrote: > >> Isn't zero filling part of the standard? I don't see why lack of > >> explicit zeroing is a warning? Looks a false warning to me. > > > > > > It is not quite as simple as this would make it sound. The elements or > > members of an aggregate (e.g.) structure type are initialized as if it > were > > an object of static storage duration (i.e., to zero) if the initializer > list > > does not contain enough initializers for all members of the aggregate > type, > > per item 21 of section 6.7.8 of n1256.pdf. However, such initialization > > does not necessarily need to zero any padding bytes that are present, > which > > may take unspecified values. Personally, I think this particular clang > > warning can be too aggressive, especially for complex structs, but on the > > other hand given the indeterminateness of padding, bzero/memset are > often a > > better choice anyway. > > > By definition, padding byte contents are unused. There is no reason > their values matter one way or another, so why do we care about the > distinction between bzero and member zero initialization? > Well, you can access them by treating the object as an array of char, and then memory sanitizer will complain about the uninitialized access, as in https://github.com/openssl/openssl/commit/f0496ad71fbacccf5a95f40d31d251bc8cf9dcfb -Ben P.S. Not really apropos of anything, but clang also has -Weverything which is just what it sounds like (as opposed to -Wall), which is a great way to expose several other warnings that Bruce will call bugs :) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Sat, 20 Feb 2016, Dimitry Andric wrote: On 19 Feb 2016, at 16:49, Alan Somers wrote: On Fri, Feb 19, 2016 at 5:24 AM, Sergey Kandaurov wrote: ... -struct nlist namelist[] = { +static struct nlist namelist[] = { #define X_TTY_NIN 0 - { "_tty_nin" }, + { .n_name = "_tty_nin", + .n_type = 0, .n_other = 0, .n_desc = 0, .n_value = 0 }, [...] You unlikely need this excessive explicit zeroization. In this case it is implicitly prezeroed. ... Yeah, it was being implicitly zeroized before. But Clang complained about the structures being only partially initialized. Since the whole point of my commit was to increase the WARNS level, I explicitly zeroed the zero fields to silence Clang. You got this warning, most likely: usr.sbin/iostat/iostat.c:122:15: error: missing field 'n_type' initializer [-Werror,-Wmissing-field-initializers] { "_tty_nin" }, ^ This warning is only produced when you use -Wall -W, and then initialize structs partially, i.e. you initialize some fields but not others. I think this is a quite reasonable warning for a high warning level. No, this more than defeats the reason for existence of C99 initializers. With C90 initializors, you sometimes had to write initializers for all the fields between the 2 that you care about, and get the order and number of al the fields right. At least you didn't have to write initializers for the trailing fields. With compiler warnings about missing initializers, you also have to write initializers for trailing fields. I forget if C99 initializers allows mixing named fields with unnamed fields, but style rules shouldn't allow it. On the other hand, if this kind of construct is used throughout the tree, and it is not seen as a big problem, we can simply silence this particular warning using -Wno-missing-field -initializers. There is already quite a list of warnings which are suppressed by default, even at WARNS=6, namely: -Wno-empty-body -Wno-format-y2k -Wno-pointer-sign -Wno-string-plus-int -Wno-unused-const-variable -Wno-unused-parameter We started using it for lists of functions for vfs, where some of the entries should be optional but all had to be supplied, in the correct order, for C90 initializers. These lists were reduced from excessively dynamic lists, and they ended up with not many optional entries, so they don't use the feature of omitting optional entries much. Bruce Bruce ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, 19 Feb 2016, Benjamin Kaduk wrote: On Fri, Feb 19, 2016 at 5:06 PM, Gleb Smirnoff wrote: On Fri, Feb 19, 2016 at 08:49:43AM -0700, Alan Somers wrote: A> On Fri, Feb 19, 2016 at 5:24 AM, Sergey Kandaurov wrote: A> Yeah, it was being implicitly zeroized before. But Clang complained A> about the structures being only partially initialized. Since the A> whole point of my commit was to increase the WARNS level, I explicitly A> zeroed the zero fields to silence Clang. Isn't zero filling part of the standard? I don't see why lack of explicit zeroing is a warning? Looks a false warning to me. It is not quite as simple as this would make it sound. The elements or members of an aggregate (e.g.) structure type are initialized as if it were an object of static storage duration (i.e., to zero) if the initializer list does not contain enough initializers for all members of the aggregate type, per item 21 of section 6.7.8 of n1256.pdf. However, such initialization does not necessarily need to zero any padding bytes that are present, which may take unspecified values. Perhaps, but then there is even less reason to expect that initializing all the struct members initialzes the padding between them. Personally, I think this particular clang warning can be too aggressive, especially for complex structs, but on the other hand given the indeterminateness of padding, bzero/memset are often a better choice anyway. It is just a bug in clang. Using auto structs with initialzers, auto structs initialized by memset/ bzero followed by initializing a few fields, are almost equally good pessimizations. Some compilers generate similar code for them (like the code shown by pluknet -- the initializer is optimized to an inlined memset followed by initializing 1 field. But doing this this on every function call is a good pessimization if the struct never changes or rarely changes or only changes slightly. Some compilers generate the worse code of keeping a static copy and generating an inline memcpy to the auto variable on every function call. 6.7.8p9 says that unnamed members have indeterminate values even after initialization, except where explicitly stated otherwise. I couldn't find anywhere "explicitly stating otherwise" that padding is initialized. It takes an explicit statements in p21 to get the unnamed members in an initializer initialized, but padding isn't even. p21 says that any unnamed struct member is initialized as if it had static storage duration with no initializer, but I couldn't find anything requiring initialization for padding in that case. Apparently, only quality of implementation prevents it being initialized with passwords from the previous program :-(. Bruce ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 3:14 PM, Benjamin Kaduk wrote: > On Fri, Feb 19, 2016 at 5:06 PM, Gleb Smirnoff wrote: >> Isn't zero filling part of the standard? I don't see why lack of >> explicit zeroing is a warning? Looks a false warning to me. > > > It is not quite as simple as this would make it sound. The elements or > members of an aggregate (e.g.) structure type are initialized as if it were > an object of static storage duration (i.e., to zero) if the initializer list > does not contain enough initializers for all members of the aggregate type, > per item 21 of section 6.7.8 of n1256.pdf. However, such initialization > does not necessarily need to zero any padding bytes that are present, which > may take unspecified values. Personally, I think this particular clang > warning can be too aggressive, especially for complex structs, but on the > other hand given the indeterminateness of padding, bzero/memset are often a > better choice anyway. By definition, padding byte contents are unused. There is no reason their values matter one way or another, so why do we care about the distinction between bzero and member zero initialization? Best, Conrad ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 3:06 PM, Gleb Smirnoff wrote: > Isn't zero filling part of the standard? I don't see why lack of > explicit zeroing is a warning? Looks a false warning to me. Yep. This is Clang being too whiney, IMO. I think a good workaround is to add -Wno-missing-field-initializers to warn flags (but otherwise keep high WARNS). Best, Conrad ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Sat, Feb 20, 2016 at 12:23:40AM +0100, Dimitry Andric wrote: D> You got this warning, most likely: D> D> usr.sbin/iostat/iostat.c:122:15: error: missing field 'n_type' initializer [-Werror,-Wmissing-field-initializers] D> { "_tty_nin" }, D> ^ D> D> This warning is only produced when you use -Wall -W, and then initialize D> structs partially, i.e. you initialize some fields but not others. I D> think this is a quite reasonable warning for a high warning level. D> D> On the other hand, if this kind of construct is used throughout the D> tree, and it is not seen as a big problem, we can simply silence this D> particular warning using -Wno-missing-field -initializers. There is D> already quite a list of warnings which are suppressed by default, even D> at WARNS=6, namely: D> D> -Wno-empty-body D> -Wno-format-y2k D> -Wno-pointer-sign D> -Wno-string-plus-int D> -Wno-unused-const-variable D> -Wno-unused-parameter I think we should add -Wno-missing-field-initializers there. We use this feature in the kernel intentionally, since it allows to write code that is less dependant on API changes. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On 19 Feb 2016, at 16:49, Alan Somers wrote: > > On Fri, Feb 19, 2016 at 5:24 AM, Sergey Kandaurov wrote: ... >>> -struct nlist namelist[] = { >>> +static struct nlist namelist[] = { >>> #define X_TTY_NIN 0 >>> - { "_tty_nin" }, >>> + { .n_name = "_tty_nin", >>> + .n_type = 0, .n_other = 0, .n_desc = 0, .n_value = 0 }, >>> [...] >> >> You unlikely need this excessive explicit zeroization. >> In this case it is implicitly prezeroed. ... > Yeah, it was being implicitly zeroized before. But Clang complained > about the structures being only partially initialized. Since the > whole point of my commit was to increase the WARNS level, I explicitly > zeroed the zero fields to silence Clang. You got this warning, most likely: usr.sbin/iostat/iostat.c:122:15: error: missing field 'n_type' initializer [-Werror,-Wmissing-field-initializers] { "_tty_nin" }, ^ This warning is only produced when you use -Wall -W, and then initialize structs partially, i.e. you initialize some fields but not others. I think this is a quite reasonable warning for a high warning level. On the other hand, if this kind of construct is used throughout the tree, and it is not seen as a big problem, we can simply silence this particular warning using -Wno-missing-field -initializers. There is already quite a list of warnings which are suppressed by default, even at WARNS=6, namely: -Wno-empty-body -Wno-format-y2k -Wno-pointer-sign -Wno-string-plus-int -Wno-unused-const-variable -Wno-unused-parameter -Dimitry signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 5:06 PM, Gleb Smirnoff wrote: > On Fri, Feb 19, 2016 at 08:49:43AM -0700, Alan Somers wrote: > A> On Fri, Feb 19, 2016 at 5:24 AM, Sergey Kandaurov > wrote: > A> Yeah, it was being implicitly zeroized before. But Clang complained > A> about the structures being only partially initialized. Since the > A> whole point of my commit was to increase the WARNS level, I explicitly > A> zeroed the zero fields to silence Clang. > > Isn't zero filling part of the standard? I don't see why lack of > explicit zeroing is a warning? Looks a false warning to me. > It is not quite as simple as this would make it sound. The elements or members of an aggregate (e.g.) structure type are initialized as if it were an object of static storage duration (i.e., to zero) if the initializer list does not contain enough initializers for all members of the aggregate type, per item 21 of section 6.7.8 of n1256.pdf. However, such initialization does not necessarily need to zero any padding bytes that are present, which may take unspecified values. Personally, I think this particular clang warning can be too aggressive, especially for complex structs, but on the other hand given the indeterminateness of padding, bzero/memset are often a better choice anyway. -Ben ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 08:49:43AM -0700, Alan Somers wrote: A> On Fri, Feb 19, 2016 at 5:24 AM, Sergey Kandaurov wrote: A> > On 18 February 2016 at 23:08, Alan Somers wrote: A> >> Author: asomers A> >> Date: Thu Feb 18 20:08:01 2016 A> >> New Revision: 295768 A> >> URL: https://svnweb.freebsd.org/changeset/base/295768 A> >> A> >> Log: A> >> Fix compiler warnings in iostat A> > A> >> Modified: head/usr.sbin/iostat/iostat.c A> >> == A> >> --- head/usr.sbin/iostat/iostat.c Thu Feb 18 19:37:39 2016 (r295767) A> >> +++ head/usr.sbin/iostat/iostat.c Thu Feb 18 20:08:01 2016 (r295768) A> >> @@ -117,30 +117,34 @@ A> >> #include A> >> #include A> >> A> >> -struct nlist namelist[] = { A> >> +static struct nlist namelist[] = { A> >> #define X_TTY_NIN 0 A> >> - { "_tty_nin" }, A> >> + { .n_name = "_tty_nin", A> >> + .n_type = 0, .n_other = 0, .n_desc = 0, .n_value = 0 }, A> >> [...] A> > A> > You unlikely need this excessive explicit zeroization. A> > In this case it is implicitly prezeroed. A> > A> > Consider these two cases: A> > A> > : #include A> > : A> > : int main(void) { A> > : struct nlist namelist[2] = {{ .n_type = 0x42 }}; A> > : return sizeof(namelist); A> > : } A> > A> > (__TEXT,__text) section A> > _main: A> > pushq%rbp A> > 0001movq%rsp, %rbp A> > 0004leaq-0x30(%rbp), %rdx A> > 0008movl$0x0, %eax A> > 000dmovl$0x6, %ecx A> > 0012movq%rdx, %rdi A> > 0015rep A> > 0016stosq A> > 0018movb$0x42, -0x28(%rbp) A> > 001cmovl$0x30, %eax A> > 0021popq%rbp A> > 0022retq A> > A> > rep stosq does zero 48 bytes, that is namelist[]. A> > A> > Or, if it is static. A> > A> > : #include A> > : A> > : int main(void) { A> > : static struct nlist namelist[2] = {{ .n_type = 0x42 }}; A> > : return sizeof(namelist); A> > : } A> > A> > (__DATA,__data) section A> > 002000 00 00 00 00 00 00 00 42 00 00 00 00 00 00 00 A> > 003000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 A> > 004000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 A> A> Yeah, it was being implicitly zeroized before. But Clang complained A> about the structures being only partially initialized. Since the A> whole point of my commit was to increase the WARNS level, I explicitly A> zeroed the zero fields to silence Clang. Isn't zero filling part of the standard? I don't see why lack of explicit zeroing is a warning? Looks a false warning to me. -- Totus tuus, Glebius. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On Fri, Feb 19, 2016 at 5:24 AM, Sergey Kandaurov wrote: > On 18 February 2016 at 23:08, Alan Somers wrote: >> Author: asomers >> Date: Thu Feb 18 20:08:01 2016 >> New Revision: 295768 >> URL: https://svnweb.freebsd.org/changeset/base/295768 >> >> Log: >> Fix compiler warnings in iostat > >> Modified: head/usr.sbin/iostat/iostat.c >> == >> --- head/usr.sbin/iostat/iostat.c Thu Feb 18 19:37:39 2016 >> (r295767) >> +++ head/usr.sbin/iostat/iostat.c Thu Feb 18 20:08:01 2016 >> (r295768) >> @@ -117,30 +117,34 @@ >> #include >> #include >> >> -struct nlist namelist[] = { >> +static struct nlist namelist[] = { >> #define X_TTY_NIN 0 >> - { "_tty_nin" }, >> + { .n_name = "_tty_nin", >> + .n_type = 0, .n_other = 0, .n_desc = 0, .n_value = 0 }, >> [...] > > You unlikely need this excessive explicit zeroization. > In this case it is implicitly prezeroed. > > Consider these two cases: > > : #include > : > : int main(void) { > : struct nlist namelist[2] = {{ .n_type = 0x42 }}; > : return sizeof(namelist); > : } > > (__TEXT,__text) section > _main: > pushq%rbp > 0001movq%rsp, %rbp > 0004leaq-0x30(%rbp), %rdx > 0008movl$0x0, %eax > 000dmovl$0x6, %ecx > 0012movq%rdx, %rdi > 0015rep > 0016stosq > 0018movb$0x42, -0x28(%rbp) > 001cmovl$0x30, %eax > 0021popq%rbp > 0022retq > > rep stosq does zero 48 bytes, that is namelist[]. > > Or, if it is static. > > : #include > : > : int main(void) { > : static struct nlist namelist[2] = {{ .n_type = 0x42 }}; > : return sizeof(namelist); > : } > > (__DATA,__data) section > 002000 00 00 00 00 00 00 00 42 00 00 00 00 00 00 00 > 003000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 004000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Yeah, it was being implicitly zeroized before. But Clang complained about the structures being only partially initialized. Since the whole point of my commit was to increase the WARNS level, I explicitly zeroed the zero fields to silence Clang. -Alan ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r295768 - head/usr.sbin/iostat
On 18 February 2016 at 23:08, Alan Somers wrote: > Author: asomers > Date: Thu Feb 18 20:08:01 2016 > New Revision: 295768 > URL: https://svnweb.freebsd.org/changeset/base/295768 > > Log: > Fix compiler warnings in iostat > Modified: head/usr.sbin/iostat/iostat.c > == > --- head/usr.sbin/iostat/iostat.c Thu Feb 18 19:37:39 2016 > (r295767) > +++ head/usr.sbin/iostat/iostat.c Thu Feb 18 20:08:01 2016 > (r295768) > @@ -117,30 +117,34 @@ > #include > #include > > -struct nlist namelist[] = { > +static struct nlist namelist[] = { > #define X_TTY_NIN 0 > - { "_tty_nin" }, > + { .n_name = "_tty_nin", > + .n_type = 0, .n_other = 0, .n_desc = 0, .n_value = 0 }, > [...] You unlikely need this excessive explicit zeroization. In this case it is implicitly prezeroed. Consider these two cases: : #include : : int main(void) { : struct nlist namelist[2] = {{ .n_type = 0x42 }}; : return sizeof(namelist); : } (__TEXT,__text) section _main: pushq%rbp 0001movq%rsp, %rbp 0004leaq-0x30(%rbp), %rdx 0008movl$0x0, %eax 000dmovl$0x6, %ecx 0012movq%rdx, %rdi 0015rep 0016stosq 0018movb$0x42, -0x28(%rbp) 001cmovl$0x30, %eax 0021popq%rbp 0022retq rep stosq does zero 48 bytes, that is namelist[]. Or, if it is static. : #include : : int main(void) { : static struct nlist namelist[2] = {{ .n_type = 0x42 }}; : return sizeof(namelist); : } (__DATA,__data) section 002000 00 00 00 00 00 00 00 42 00 00 00 00 00 00 00 003000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 004000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -- wbr, pluknet ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"