Re: [patch 07/26] Add INIT_ARRAY() to kernel.h

Previous thread: [patch 26/26] Linux Kernel Markers - Use Immediate Values by Mathieu Desnoyers on Thursday, January 24, 2008 - 1:27 pm. (1 message)

Next thread: [patch 03/26] Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig by Mathieu Desnoyers on Thursday, January 24, 2008 - 1:27 pm. (5 messages)
From: Mathieu Desnoyers
Date: Thursday, January 24, 2008 - 1:27 pm

Add initialization of an array, which needs brackets that would pollute kernel
code, to kernel.h. It is used to declare arguments passed as function parameters
such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================
--- linux-2.6-lttng.mm.orig/include/linux/kernel.h	2008-01-24 14:10:54.000000000 -0500
+++ linux-2.6-lttng.mm/include/linux/kernel.h	2008-01-24 14:23:06.000000000 -0500
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type [len]) { [0 ... (len)-1] = (val) })
+
 #endif

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--

From: Jan Engelhardt
Date: Thursday, January 24, 2008 - 1:39 pm

text_poke(addr, (unsigned char[]){[0 ... len-1] = val});

  #define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)})

You can use type[].
--

From: Mathieu Desnoyers
Date: Thursday, January 24, 2008 - 1:54 pm

Add initialization of an array, which needs brackets that would pollute kernel
code, to kernel.h. It is used to declare arguments passed as function parameters
such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Changelog :
- [len] -> []
- Remove whitespaces.
- Fixed Jan's suggestion which was not checkpatch.pl-safe.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Jan Engelhardt <jengelh@computergmbh.de>
---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================
--- linux-2.6-lttng.mm.orig/include/linux/kernel.h	2008-01-24 15:53:04.000000000 -0500
+++ linux-2.6-lttng.mm/include/linux/kernel.h	2008-01-24 15:53:25.000000000 -0500
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type[]) {[0 ... (len)-1] = (val)})
+
 #endif

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--

From: Jan Engelhardt
Date: Thursday, January 24, 2008 - 2:08 pm

diff -dpru /dev/null <(echo '  #define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)}) ') | scripts/checkpatch.pl -
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 2 lines checked

Where did checkpatch play unfair again on your side? :)

--

From: Mathieu Desnoyers
Date: Thursday, January 24, 2008 - 2:18 pm

Hrm, I don't know why checkpatch does not notice the problem when
invoked with your command, but creating a patch with your change results
in :

compudj@dijkstra:~/git/morestable/linux-2.6-lttng.mm$ scripts/checkpatch.pl patches/test.patch 
ERROR: need a space before the open brace '{'
#27: FILE: include/linux/kernel.h:426:
+#define INIT_ARRAY(type, val, len) ((type[]){[0 ... (len)-1] = (val)})

total: 1 errors, 0 warnings, 6 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

but I guess we are deep down in nitpicking land ;)

Cheers,

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--

From: Randy Dunlap
Date: Thursday, January 24, 2008 - 1:58 pm

Hi Mathieu,

Maybe you have explained this previously, but please give a short
explanation of "brackets that would pollute kernel code".


---
~Randy
--

From: Mathieu Desnoyers
Date: Thursday, January 24, 2008 - 2:04 pm

Try opening such code in vim, and the syntax highlighting gets all
messed up :-/

The other option is to do the *right thing* and fix vim, of course, but
this change won't be in distros for a while.


-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--

From: Stefan Richter
Date: Thursday, January 24, 2008 - 3:02 pm

Is it about unbalanced brackets?

Anyway, if you add it because other kinds of initializers (which ones?)
aren't displayed nicely in syntax highlighting editors, just say so in
the changelog.  Your log isn't informative at all.
-- 
Stefan Richter
-=====-==--- ---= ==---
http://arcgraph.de/sr/

--

From: Mathieu Desnoyers
Date: Thursday, January 24, 2008 - 3:10 pm

Array initializers in the body of a function causes unbalanced brackets which
syntax highlighting editors such as vim have problems with. Fix this by creating
the INIT_ARRAY() macro.

It is used to declare arguments passed as function parameters such as:
text_poke(addr, INIT_ARRAY(unsigned char, 0xf0, len), len);

Changelog :
- [len] -> []
- Remove whitespaces.
- Fixed Jan's suggestion which was not checkpatch.pl-safe.
- More informative patch header.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Jan Engelhardt <jengelh@computergmbh.de>
CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 include/linux/kernel.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng.mm/include/linux/kernel.h
===================================================================
--- linux-2.6-lttng.mm.orig/include/linux/kernel.h	2008-01-24 15:53:04.000000000 -0500
+++ linux-2.6-lttng.mm/include/linux/kernel.h	2008-01-24 15:53:25.000000000 -0500
@@ -423,4 +423,6 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+#define INIT_ARRAY(type, val, len) ((type[]) {[0 ... (len)-1] = (val)})
+
 #endif

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--

From: Alexey Dobriyan
Date: Thursday, January 24, 2008 - 3:50 pm

While we are at useless macro territory, let's name it MAKE_ARRAY because
it's exactly what it does.
--

From: H. Peter Anvin
Date: Thursday, January 24, 2008 - 4:04 pm

Let's get this straight...

You're suggesting introducing a pointless, ugly macro into the kernel 
because of a vim bug?

*Plonk!*

	-hpa
--

From: Mathieu Desnoyers
Date: Friday, January 25, 2008 - 6:14 am

heh, yup, I guess we should drop this patch. As you point out, it makes
no sense.


-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--

From: Jan Engelhardt
Date: Friday, January 25, 2008 - 1:03 am

Then we'd also have to fix dcache.h:

#define DCACHE_NFSFS_RENAMED  0x0002    /* this dentry has been "silly 
                                         * renamed" and has to be 
                                         * deleted on the last dput() 
                                         */ 

by adding \ at the end of the comments, to unbreak cooledit and mc.
(The problem is not the comment, but the seemingly stray " in the 2nd line)
--

From: H. Peter Anvin
Date: Thursday, January 24, 2008 - 4:03 pm

"brackets which would pollute kernel code"???

	-hpa
--

Previous thread: [patch 26/26] Linux Kernel Markers - Use Immediate Values by Mathieu Desnoyers on Thursday, January 24, 2008 - 1:27 pm. (1 message)

Next thread: [patch 03/26] Move Kconfig.instrumentation to arch/Kconfig and init/Kconfig by Mathieu Desnoyers on Thursday, January 24, 2008 - 1:27 pm. (5 messages)