Re: CVS commit: src/sys/fs/tmpfs
On Sat, May 14, 2011 at 10:34:05AM +0200, Marc Balmer wrote: > What is the current state of C99 vs. older Cs? Do all arches / > compilers we have support C99? We have lost the playstation2 port, because we don't have a working C99 compiler for it (so a -current kernel can not be compiled). Martin
Change the subject (defaulting to c99 in userland) was: CVS commit: src/sys/fs/tmpfs
On Sat, May 14, 2011 at 04:45:07PM +, David Holland wrote: > On Sat, May 14, 2011 at 12:07:20PM -0400, Christos Zoulas wrote: > > If we are going to be compiling the kernel in c99 mode, then I > > suggest that we do the same for userland instead of turning it on > > for userland piecemeal. > > +1 > > is there anything we expect to break? It would be nice if this thread was not just on source-changes-d. Bernd
Re: CVS commit: src/sys/fs/tmpfs
14.05.2011, 10:38, "Masao Uebayashi" : > I disagree. If variables are declared in the middle of context, those > variables have narrower contexts. Narrowing context is always a win > IMO. That's true only if you don't use gotos. Otherwise, you might jump past an initialization point with obvious consequences. If I remember correctly, a compiler reports an error only for variable length arrays in this case. -- Alex
Re: CVS commit: src/sys/fs/tmpfs
On Sat, May 14, 2011 at 12:07:20PM -0400, Christos Zoulas wrote: > If we are going to be compiling the kernel in c99 mode, then I > suggest that we do the same for userland instead of turning it on > for userland piecemeal. +1 is there anything we expect to break? -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/crypto/rijndael
On 5/14/11 12:31 PM, Christos Zoulas wrote: On May 14, 12:29pm, j...@julipedia.org (Julio Merino) wrote: -- Subject: Re: CVS commit: src/sys/crypto/rijndael |> Declare for-loop control variable outside of the for statement to prevent |> a warning and therefore fix the build. | | Ah! I just saw your warns=4 change. I presume my 'fix' goes against | it, right? Yes, you should revert it. Kernel code is supposed to use c99 features. OK; I'll do a test build before submitting the revert. I just realized that my tree already had your change when I committed this, yet the build still failed.
Re: CVS commit: src/sys/crypto/rijndael
On May 14, 12:29pm, j...@julipedia.org (Julio Merino) wrote: -- Subject: Re: CVS commit: src/sys/crypto/rijndael | > Declare for-loop control variable outside of the for statement to prevent | > a warning and therefore fix the build. | | Ah! I just saw your warns=4 change. I presume my 'fix' goes against | it, right? Yes, you should revert it. Kernel code is supposed to use c99 features. christos
Re: CVS commit: src/sys/crypto/rijndael
On 5/14/11 12:27 PM, Julio Merino wrote: Module Name:src Committed By: jmmv Date: Sat May 14 16:27:50 UTC 2011 Modified Files: src/sys/crypto/rijndael: rijndael-api-fst.c Log Message: Declare for-loop control variable outside of the for statement to prevent a warning and therefore fix the build. Ah! I just saw your warns=4 change. I presume my 'fix' goes against it, right?
Re: CVS commit: src/sys/crypto/rijndael
On May 14, 1:02pm, hann...@eis.cs.tu-bs.de (=?iso-8859-1?Q?J=FCrgen_Hannken-Illjes?=) wrote: -- Subject: Re: CVS commit: src/sys/crypto/rijndael | This breaks in src/regress/sys/crypto/rijndael: | | # compile rijndael/rijndael-api-fst.o | 486--netbsdelf-gcc -O2 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpo= | inter-arith -Wno-sign-compare -no-traditional -Wa,--fatal-warnings -Werror= |--sysroot=3D/work/build/obj/dest.i386 -I/work/build/src/sys "-Dpanic(x)= | =3Dabort()" -c src/sys/crypto/rijndael/rijndael-api-fst.c | src/sys/crypto/rijndael/rijndael-api-fst.c: In function 'xor16': | src/sys/crypto/rijndael/rijndael-api-fst.c:57: error: 'for' loop initial de= | claration used outside C99 mode Fixed, thanks! christos
Re: CVS commit: src/sys/fs/tmpfs
On May 14, 12:00pm, rm...@netbsd.org (Mindaugas Rasiukevicius) wrote:
-- Subject: Re: CVS commit: src/sys/fs/tmpfs
| Benefit is code readability. It is easier to track the variables when
| they are defined and initialised in the beginning of context.
|
| If code is longer and/or complex - it likely has loops or conditional
| statements, and variables can be defined in the beginning of *their*
| context (basically, after { bracket). This is very encouraged.
|
| For other cases, compilers do a good job anyway (if you do not believe
| me - check with objdump) and there is no need to hurt code readability.
This whole discussion misses the point. The reason I changed the
code back to regular c from c99 is because the code did not compile.
This happened because rump did not pass -std=gnu99 in the compiler
flags. Now I did 'for (size_t i = 0;' in sys/crypto and the regression
tests failed. If we are going to be compiling the kernel in c99
mode, then I suggest that we do the same for userland instead of
turning it on for userland piecemeal.
christos
Re: CVS commit: src/sys/crypto/rijndael
On May 14, 2011, at 3:59 AM, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sat May 14 01:59:19 UTC 2011 > > Modified Files: > src/sys/crypto/rijndael: rijndael-api-fst.c > > Log Message: > - don't assume aligned buffers. > - little KNF > > > To generate a diff of this commit: > cvs rdiff -u -r1.21 -r1.22 src/sys/crypto/rijndael/rijndael-api-fst.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > This breaks in src/regress/sys/crypto/rijndael: # compile rijndael/rijndael-api-fst.o 486--netbsdelf-gcc -O2 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-sign-compare -no-traditional -Wa,--fatal-warnings -Werror --sysroot=/work/build/obj/dest.i386 -I/work/build/src/sys "-Dpanic(x)=abort()" -c src/sys/crypto/rijndael/rijndael-api-fst.c src/sys/crypto/rijndael/rijndael-api-fst.c: In function 'xor16': src/sys/crypto/rijndael/rijndael-api-fst.c:57: error: 'for' loop initial declaration used outside C99 mode -- Jürgen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
Masao Uebayashi wrote:
> >> The kernel explicitly allows C99 and actually C99 is encouraged.
> >> So that should reverted :)
> >
> > Generally - C99 is encouraged. However, I disagree that variables
> > should be declared in the middle of context (for a minimum scope),
> > unless there is a *clear* benefit. Otherwise, it makes code harder
> > to read, especially if code fragment is long and/or complex.
>
> I disagree. If variables are declared in the middle of context, those
> variables have narrower contexts. Narrowing context is always a win
> IMO.
>
> I'd like to hear the benefit not doing this (== the old convention).
Benefit is code readability. It is easier to track the variables when
they are defined and initialised in the beginning of context.
If code is longer and/or complex - it likely has loops or conditional
statements, and variables can be defined in the beginning of *their*
context (basically, after { bracket). This is very encouraged.
For other cases, compilers do a good job anyway (if you do not believe
me - check with objdump) and there is no need to hurt code readability.
> >
> > Benefits could be, e.g. use of const or limitation of the variable
> > scope for performance sensitive code, also avoiding of #ifdefs, etc.
> >
> > In this case, I used for (int i = 0; ...) because the loop was in
> > the beginning of context and #ifdef DEBUG-only.
> >
> > --
> > Mindaugas
> >
--
Mindaugas
Re: CVS commit: src/sys/fs/tmpfs
On Sat, 14 May 2011, Marc Balmer wrote: > What is the current state of C99 vs. older Cs? Do all arches / > compilers we have support C99? I assume gcc, llvm/clang are safe, but > what about pcc wrt C99? > > I'd like a short clarification here, since this might influence my > coding... tnx. pcc is a C99 compiler (with some gcc compatibility) which is still under development, though C99 feature support is complete. pcc is capable of building large parts of userland (I am running with /bin, /sbin and /usr/bin currently, and am going to install /usr/sbin soon), plus i386 kernels though there are still bugs to track down (eg no system crash but a build.sh failed, I think due to some corrupted files..) I'm thinking that though we have some support for C99 in tree, the 'official' position is that llvm/clang and pcc are not yet supported (eg there has been no such announcement of support, llvm/clang source is not yet in tree and the in-tree pcc is a year out of date). So IMO, apart from style issues (which it would be nice to update the share/misc/style document with), it should be safe to use any C99 features and, excepting some of the build tools which may be needed for bootstrapping, I don't think its useful to restrict ourselves to an older standard.. iain
Re: CVS commit: src/sys/fs/tmpfs
Am 14.05.11 10:45, schrieb Anders Magnusson: [...] >> What is the current state of C99 vs. older Cs? Do all arches / >> compilers we have support C99? I assume gcc, llvm/clang are safe, but >> what about pcc wrt C99? > pcc should be C99 compliant. If something do not work as expected, I'll > fix it. cool! thanks for the update. > > -- Ragge [...]
Re: CVS commit: src/sys/fs/tmpfs
On 05/14/2011 10:34 AM, Marc Balmer wrote:
Am 10.05.11 02:34, schrieb Matt Thomas:
Module Name:src
Committed By: matt
Date: Tue May 10 00:34:26 UTC 2011
Modified Files:
src/sys/fs/tmpfs: tmpfs_vnops.c
Log Message:
yes, more C99 please (back out previous change).
After this committ/back-out/back-out-pf-the-back-out fest I have a
technical question:
What is the current state of C99 vs. older Cs? Do all arches /
compilers we have support C99? I assume gcc, llvm/clang are safe, but
what about pcc wrt C99?
pcc should be C99 compliant. If something do not work as expected, I'll
fix it.
-- Ragge
I'd like a short clarification here, since this might influence my
coding... tnx.
To generate a diff of this commit:
cvs rdiff -u -r1.79 -r1.80 src/sys/fs/tmpfs/tmpfs_vnops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/fs/tmpfs/tmpfs_vnops.c
diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.79 src/sys/fs/tmpfs/tmpfs_vnops.c:1.80
--- src/sys/fs/tmpfs/tmpfs_vnops.c:1.79 Sun May 8 00:03:35 2011
+++ src/sys/fs/tmpfs/tmpfs_vnops.c Tue May 10 00:34:26 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35 christos Exp $
*/
+/* $NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt Exp $*/
/*
* Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35 christos Exp
$");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt Exp
$");
#include
#include
@@ -1466,8 +1466,7 @@
#if defined(DEBUG)
if (!error&& pgs) {
- int i;
- for (i = 0; i< npages; i++) {
+ for (int i = 0; i< npages; i++) {
KASSERT(pgs[i] != NULL);
}
}
Re: CVS commit: src/sys/fs/tmpfs
Am 10.05.11 02:34, schrieb Matt Thomas:
> Module Name: src
> Committed By: matt
> Date: Tue May 10 00:34:26 UTC 2011
>
> Modified Files:
> src/sys/fs/tmpfs: tmpfs_vnops.c
>
> Log Message:
> yes, more C99 please (back out previous change).
After this committ/back-out/back-out-pf-the-back-out fest I have a
technical question:
What is the current state of C99 vs. older Cs? Do all arches /
compilers we have support C99? I assume gcc, llvm/clang are safe, but
what about pcc wrt C99?
I'd like a short clarification here, since this might influence my
coding... tnx.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.79 -r1.80 src/sys/fs/tmpfs/tmpfs_vnops.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>
>
>
> Modified files:
>
> Index: src/sys/fs/tmpfs/tmpfs_vnops.c
> diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.79
> src/sys/fs/tmpfs/tmpfs_vnops.c:1.80
> --- src/sys/fs/tmpfs/tmpfs_vnops.c:1.79 Sun May 8 00:03:35 2011
> +++ src/sys/fs/tmpfs/tmpfs_vnops.cTue May 10 00:34:26 2011
> @@ -1,4 +1,4 @@
> -/* $NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35 christos Exp $
> */
> +/* $NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt Exp $*/
>
> /*
> * Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
> @@ -35,7 +35,7 @@
> */
>
> #include
> -__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.79 2011/05/08 00:03:35
> christos Exp $");
> +__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.80 2011/05/10 00:34:26 matt
> Exp $");
>
> #include
> #include
> @@ -1466,8 +1466,7 @@
>
> #if defined(DEBUG)
> if (!error && pgs) {
> - int i;
> - for (i = 0; i < npages; i++) {
> + for (int i = 0; i < npages; i++) {
> KASSERT(pgs[i] != NULL);
> }
> }
>
