linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init))
@ 2014-07-31 23:47 Josh Triplett
  2014-07-31 23:47 ` [PATCH 2/5] raid: Require designated initialization of structures Josh Triplett
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Josh Triplett @ 2014-07-31 23:47 UTC (permalink / raw)
  To: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Neil Brown,
	Steven Rostedt, linux-api, linux-fsdevel, linux-kbuild,
	linux-kernel, linux-raid, linux-sparse

GCC 4.10 and newer, and Sparse, supports
__attribute__((designated_init)), which marks a structure as requiring
a designated initializer rather than a positional one.  This helps
reduce churn and errors when used with _ops structures and similar
structures designed for future extension.

Add a wrapper __designated_init, which turns into
__attribute__((designated_init)) for Sparse or sufficiently new GCC.
Enable the corresponding warning as an error.

The following semantic patch can help mark structures containing
function pointers as requiring designated initializers:

@@
identifier I, f;
type T;
@@

struct I {
	...
	T (*f)(...);
	...
}
+ __designated_init
;

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 Makefile                      | 3 +++
 include/linux/compiler-gcc4.h | 4 ++++
 include/linux/compiler.h      | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/Makefile b/Makefile
index f6a7794..83773c2 100644
--- a/Makefile
+++ b/Makefile
@@ -744,6 +744,9 @@ KBUILD_CFLAGS   += $(call cc-option,-Werror=strict-prototypes)
 # Prohibit date/time macros, which would make the build non-deterministic
 KBUILD_CFLAGS   += $(call cc-option,-Werror=date-time)
 
+# Disallow positional initialization of designated structs
+KBUILD_CFLAGS += $(call cc-option,-Werror=designated-init)
+
 # use the deterministic mode of AR if available
 KBUILD_ARFLAGS := $(call ar-option,D)
 
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 2507fd2..5cd3c26 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -85,4 +85,8 @@
 #if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600)
 #define __HAVE_BUILTIN_BSWAP16__
 #endif
+
+#if GCC_VERSION >= 41000 || defined(__CHECKER__)
+#define __designated_init __attribute__((designated_init))
+#endif
 #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index d5ad7b1..c2334b2 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -266,6 +266,11 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
 #define __always_inline inline
 #endif
 
+/* Marks a struct as requiring designated initializers, never positional. */
+#ifndef __designated_init
+#define __designated_init
+#endif
+
 #endif /* __KERNEL__ */
 
 /*
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] raid: Require designated initialization of structures
  2014-07-31 23:47 [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Josh Triplett
@ 2014-07-31 23:47 ` Josh Triplett
  2014-08-01  1:10   ` NeilBrown
  2014-07-31 23:47 ` [PATCH 3/5] fs: " Josh Triplett
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Josh Triplett @ 2014-07-31 23:47 UTC (permalink / raw)
  To: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Neil Brown,
	Steven Rostedt, linux-api, linux-fsdevel, linux-kbuild,
	linux-kernel, linux-raid, linux-sparse

Mark raid6_calls and other structures containing function pointers with
__designated_init.  Fix implementations in lib/raid6/ to use designated
initializers; this also simplifies those initializers using the default
initialization of fields to 0.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/raid/pq.h    |  4 ++--
 include/linux/raid/xor.h   |  2 +-
 include/linux/raid_class.h |  2 +-
 lib/raid6/altivec.uc       |  7 +++----
 lib/raid6/avx2.c           | 24 ++++++++++++------------
 lib/raid6/int.uc           |  6 ++----
 lib/raid6/mmx.c            | 14 ++++++--------
 lib/raid6/neon.c           |  7 +++----
 lib/raid6/sse1.c           | 16 ++++++++--------
 lib/raid6/sse2.c           | 24 ++++++++++++------------
 lib/raid6/tilegx.uc        |  6 ++----
 11 files changed, 52 insertions(+), 60 deletions(-)

diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h
index 73069cb..2147bff 100644
--- a/include/linux/raid/pq.h
+++ b/include/linux/raid/pq.h
@@ -75,7 +75,7 @@ struct raid6_calls {
 	int  (*valid)(void);	/* Returns 1 if this routine set is usable */
 	const char *name;	/* Name of this routine set */
 	int prefer;		/* Has special performance attribute */
-};
+} __designated_init;
 
 /* Selected algorithm */
 extern struct raid6_calls raid6_call;
@@ -109,7 +109,7 @@ struct raid6_recov_calls {
 	int  (*valid)(void);
 	const char *name;
 	int priority;
-};
+} __designated_init;
 
 extern const struct raid6_recov_calls raid6_recov_intx1;
 extern const struct raid6_recov_calls raid6_recov_ssse3;
diff --git a/include/linux/raid/xor.h b/include/linux/raid/xor.h
index 5a21095..c7df59f 100644
--- a/include/linux/raid/xor.h
+++ b/include/linux/raid/xor.h
@@ -17,6 +17,6 @@ struct xor_block_template {
 		     unsigned long *, unsigned long *);
 	void (*do_5)(unsigned long, unsigned long *, unsigned long *,
 		     unsigned long *, unsigned long *, unsigned long *);
-};
+} __designated_init;
 
 #endif
diff --git a/include/linux/raid_class.h b/include/linux/raid_class.h
index 31e1ff6..603af94 100644
--- a/include/linux/raid_class.h
+++ b/include/linux/raid_class.h
@@ -16,7 +16,7 @@ struct raid_function_template {
 	int (*is_raid)(struct device *);
 	void (*get_resync)(struct device *);
 	void (*get_state)(struct device *);
-};
+} __designated_init;
 
 enum raid_state {
 	RAID_STATE_UNKNOWN = 0,
diff --git a/lib/raid6/altivec.uc b/lib/raid6/altivec.uc
index 7cc12b5..4ff138c 100644
--- a/lib/raid6/altivec.uc
+++ b/lib/raid6/altivec.uc
@@ -118,10 +118,9 @@ int raid6_have_altivec(void)
 #endif
 
 const struct raid6_calls raid6_altivec$# = {
-	raid6_altivec$#_gen_syndrome,
-	raid6_have_altivec,
-	"altivecx$#",
-	0
+	.gen_syndrome = raid6_altivec$#_gen_syndrome,
+	.valid = raid6_have_altivec,
+	.name = "altivecx$#",
 };
 
 #endif /* CONFIG_ALTIVEC */
diff --git a/lib/raid6/avx2.c b/lib/raid6/avx2.c
index bc3b1dd..e56fa06 100644
--- a/lib/raid6/avx2.c
+++ b/lib/raid6/avx2.c
@@ -88,10 +88,10 @@ static void raid6_avx21_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_avx2x1 = {
-	raid6_avx21_gen_syndrome,
-	raid6_have_avx2,
-	"avx2x1",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_avx21_gen_syndrome,
+	.valid = raid6_have_avx2,
+	.name = "avx2x1",
+	.prefer = 1,		/* Has cache hints */
 };
 
 /*
@@ -149,10 +149,10 @@ static void raid6_avx22_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_avx2x2 = {
-	raid6_avx22_gen_syndrome,
-	raid6_have_avx2,
-	"avx2x2",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_avx22_gen_syndrome,
+	.valid = raid6_have_avx2,
+	.name = "avx2x2",
+	.prefer = 1,		/* Has cache hints */
 };
 
 #ifdef CONFIG_X86_64
@@ -241,10 +241,10 @@ static void raid6_avx24_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_avx2x4 = {
-	raid6_avx24_gen_syndrome,
-	raid6_have_avx2,
-	"avx2x4",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_avx24_gen_syndrome,
+	.valid = raid6_have_avx2,
+	.name = "avx2x4",
+	.prefer = 1,		/* Has cache hints */
 };
 #endif
 
diff --git a/lib/raid6/int.uc b/lib/raid6/int.uc
index 5b50f8d..35ad01a 100644
--- a/lib/raid6/int.uc
+++ b/lib/raid6/int.uc
@@ -108,10 +108,8 @@ static void raid6_int$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_intx$# = {
-	raid6_int$#_gen_syndrome,
-	NULL,		/* always valid */
-	"int" NSTRING "x$#",
-	0
+	.gen_syndrome = raid6_int$#_gen_syndrome,
+	.name = "int" NSTRING "x$#",
 };
 
 #endif
diff --git a/lib/raid6/mmx.c b/lib/raid6/mmx.c
index 590c71c..cdd7d02 100644
--- a/lib/raid6/mmx.c
+++ b/lib/raid6/mmx.c
@@ -75,10 +75,9 @@ static void raid6_mmx1_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_mmxx1 = {
-	raid6_mmx1_gen_syndrome,
-	raid6_have_mmx,
-	"mmxx1",
-	0
+	.gen_syndrome = raid6_mmx1_gen_syndrome,
+	.valid = raid6_have_mmx,
+	.name = "mmxx1",
 };
 
 /*
@@ -133,10 +132,9 @@ static void raid6_mmx2_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_mmxx2 = {
-	raid6_mmx2_gen_syndrome,
-	raid6_have_mmx,
-	"mmxx2",
-	0
+	.gen_syndrome = raid6_mmx2_gen_syndrome,
+	.valid = raid6_have_mmx,
+	.name = "mmxx2",
 };
 
 #endif
diff --git a/lib/raid6/neon.c b/lib/raid6/neon.c
index 36ad470..99100dd 100644
--- a/lib/raid6/neon.c
+++ b/lib/raid6/neon.c
@@ -41,10 +41,9 @@
 		kernel_neon_end();					\
 	}								\
 	struct raid6_calls const raid6_neonx ## _n = {			\
-		raid6_neon ## _n ## _gen_syndrome,			\
-		raid6_have_neon,					\
-		"neonx" #_n,						\
-		0							\
+		.gen_syndrome = raid6_neon ## _n ## _gen_syndrome,	\
+		.valid = raid6_have_neon,				\
+		.name = "neonx" #_n,					\
 	}
 
 static int raid6_have_neon(void)
diff --git a/lib/raid6/sse1.c b/lib/raid6/sse1.c
index f762971..a9de46e 100644
--- a/lib/raid6/sse1.c
+++ b/lib/raid6/sse1.c
@@ -91,10 +91,10 @@ static void raid6_sse11_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_sse1x1 = {
-	raid6_sse11_gen_syndrome,
-	raid6_have_sse1_or_mmxext,
-	"sse1x1",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_sse11_gen_syndrome,
+	.valid = raid6_have_sse1_or_mmxext,
+	.name = "sse1x1",
+	.prefer = 1,		/* Has cache hints */
 };
 
 /*
@@ -153,10 +153,10 @@ static void raid6_sse12_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_sse1x2 = {
-	raid6_sse12_gen_syndrome,
-	raid6_have_sse1_or_mmxext,
-	"sse1x2",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_sse12_gen_syndrome,
+	.valid = raid6_have_sse1_or_mmxext,
+	.name = "sse1x2",
+	.prefer = 1,		/* Has cache hints */
 };
 
 #endif
diff --git a/lib/raid6/sse2.c b/lib/raid6/sse2.c
index 85b82c8..cd262518aa 100644
--- a/lib/raid6/sse2.c
+++ b/lib/raid6/sse2.c
@@ -89,10 +89,10 @@ static void raid6_sse21_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_sse2x1 = {
-	raid6_sse21_gen_syndrome,
-	raid6_have_sse2,
-	"sse2x1",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_sse21_gen_syndrome,
+	.valid = raid6_have_sse2,
+	.name = "sse2x1",
+	.prefer = 1,		/* Has cache hints */
 };
 
 /*
@@ -151,10 +151,10 @@ static void raid6_sse22_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_sse2x2 = {
-	raid6_sse22_gen_syndrome,
-	raid6_have_sse2,
-	"sse2x2",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_sse22_gen_syndrome,
+	.valid = raid6_have_sse2,
+	.name = "sse2x2",
+	.prefer = 1,		/* Has cache hints */
 };
 
 #ifdef CONFIG_X86_64
@@ -249,10 +249,10 @@ static void raid6_sse24_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_sse2x4 = {
-	raid6_sse24_gen_syndrome,
-	raid6_have_sse2,
-	"sse2x4",
-	1			/* Has cache hints */
+	.gen_syndrome = raid6_sse24_gen_syndrome,
+	.valid = raid6_have_sse2,
+	.name = "sse2x4",
+	.prefer = 1,		/* Has cache hints */
 };
 
 #endif /* CONFIG_X86_64 */
diff --git a/lib/raid6/tilegx.uc b/lib/raid6/tilegx.uc
index e7c2945..3077722 100644
--- a/lib/raid6/tilegx.uc
+++ b/lib/raid6/tilegx.uc
@@ -79,8 +79,6 @@ void raid6_tilegx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
 }
 
 const struct raid6_calls raid6_tilegx$# = {
-	raid6_tilegx$#_gen_syndrome,
-	NULL,
-	"tilegx$#",
-	0
+	.gen_syndrome = raid6_tilegx$#_gen_syndrome,
+	.name = "tilegx$#",
 };
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] fs: Require designated initialization of structures
  2014-07-31 23:47 [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Josh Triplett
  2014-07-31 23:47 ` [PATCH 2/5] raid: Require designated initialization of structures Josh Triplett
@ 2014-07-31 23:47 ` Josh Triplett
  2014-07-31 23:47 ` [PATCH 4/5] ftrace: " Josh Triplett
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Josh Triplett @ 2014-07-31 23:47 UTC (permalink / raw)
  To: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Neil Brown,
	Steven Rostedt, linux-api, linux-fsdevel, linux-kbuild,
	linux-kernel, linux-raid, linux-sparse

Mark various filesystem structures with __designated_init.  Fix the one
and only instance of positional initialization of those structures.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 fs/ioctl.c         |  2 +-
 include/linux/fs.h | 45 +++++++++++++++++++++++----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 8ac3fad..2151968 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -174,7 +174,7 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
 {
 	struct fiemap fiemap;
 	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
-	struct fiemap_extent_info fieinfo = { 0, };
+	struct fiemap_extent_info fieinfo = { };
 	struct inode *inode = file_inode(filp);
 	struct super_block *sb = inode->i_sb;
 	u64 len;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e11d60c..cec614b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -366,7 +366,7 @@ struct address_space_operations {
 	int (*swap_activate)(struct swap_info_struct *sis, struct file *file,
 				sector_t *span);
 	void (*swap_deactivate)(struct file *file);
-};
+} __designated_init;
 
 extern const struct address_space_operations empty_aops;
 
@@ -401,7 +401,7 @@ struct address_space {
 	spinlock_t		private_lock;	/* for use by the address_space */
 	struct list_head	private_list;	/* ditto */
 	void			*private_data;	/* ditto */
-} __attribute__((aligned(sizeof(long))));
+} __designated_init __attribute__((aligned(sizeof(long))));
 	/*
 	 * On most architectures that alignment is already the case; but
 	 * must be enforced here for CRIS, to let the least significant bit
@@ -444,7 +444,7 @@ struct block_device {
 	int			bd_fsfreeze_count;
 	/* Mutex for freeze */
 	struct mutex		bd_fsfreeze_mutex;
-};
+} __designated_init;
 
 /*
  * Radix-tree tags, for tagging dirty and writeback pages within the pagecache
@@ -588,7 +588,7 @@ struct inode {
 #endif
 
 	void			*i_private; /* fs or device private pointer */
-};
+} __designated_init;
 
 static inline int inode_unhashed(struct inode *inode)
 {
@@ -719,7 +719,7 @@ struct fown_struct {
 	enum pid_type pid_type;	/* Kind of process group SIGIO should be sent to */
 	kuid_t uid, euid;	/* uid/euid of process setting the owner */
 	int signum;		/* posix.1b rt signal to be delivered on IO */
-};
+} __designated_init;
 
 /*
  * Track a single file's readahead state
@@ -733,7 +733,7 @@ struct file_ra_state {
 	unsigned int ra_pages;		/* Maximum readahead window */
 	unsigned int mmap_miss;		/* Cache miss stat for mmap accesses */
 	loff_t prev_pos;		/* Cache last read() position */
-};
+} __designated_init;
 
 /*
  * Check if @index falls in the readahead windows.
@@ -781,14 +781,15 @@ struct file {
 	struct list_head	f_tfile_llink;
 #endif /* #ifdef CONFIG_EPOLL */
 	struct address_space	*f_mapping;
-} __attribute__((aligned(4)));	/* lest something weird decides that 2 is OK */
+} __designated_init __attribute__((aligned(4)));
+/* lest something weird decides that 2 is OK */
 
 struct file_handle {
 	__u32 handle_bytes;
 	int handle_type;
 	/* file identifier */
 	unsigned char f_handle[0];
-};
+} __designated_init;
 
 static inline struct file *get_file(struct file *f)
 {
@@ -838,7 +839,7 @@ typedef struct files_struct *fl_owner_t;
 struct file_lock_operations {
 	void (*fl_copy_lock)(struct file_lock *, struct file_lock *);
 	void (*fl_release_private)(struct file_lock *);
-};
+} __designated_init;
 
 struct lock_manager_operations {
 	int (*lm_compare_owner)(struct file_lock *, struct file_lock *);
@@ -847,7 +848,7 @@ struct lock_manager_operations {
 	int (*lm_grant)(struct file_lock *, struct file_lock *, int);
 	void (*lm_break)(struct file_lock *);
 	int (*lm_change)(struct file_lock **, int);
-};
+} __designated_init;
 
 struct lock_manager {
 	struct list_head list;
@@ -909,7 +910,7 @@ struct file_lock {
 			int state;		/* state of grant or error if -ve */
 		} afs;
 	} fl_u;
-};
+} __designated_init;
 
 /* The following constant reflects the upper bound of the file/locking space */
 #ifndef OFFSET_MAX
@@ -1112,7 +1113,7 @@ struct fasync_struct {
 	struct fasync_struct	*fa_next; /* singly linked list */
 	struct file		*fa_file;
 	struct rcu_head		fa_rcu;
-};
+} __designated_init;
 
 #define FASYNC_MAGIC 0x4601
 
@@ -1170,7 +1171,7 @@ struct sb_writers {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lockdep_map	lock_map[SB_FREEZE_LEVELS];
 #endif
-};
+} __designated_init;
 
 struct super_block {
 	struct list_head	s_list;		/* Keep this first */
@@ -1258,7 +1259,7 @@ struct super_block {
 	struct list_lru		s_dentry_lru ____cacheline_aligned_in_smp;
 	struct list_lru		s_inode_lru ____cacheline_aligned_in_smp;
 	struct rcu_head		rcu;
-};
+} __designated_init;
 
 extern struct timespec current_fs_time(struct super_block *sb);
 
@@ -1410,7 +1411,7 @@ struct fiemap_extent_info {
 	unsigned int fi_extents_max;	/* Size of fiemap_extent array */
 	struct fiemap_extent __user *fi_extents_start; /* Start of
 							fiemap_extent array */
-};
+} __designated_init;
 int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
 			    u64 phys, u64 len, u32 flags);
 int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
@@ -1441,7 +1442,7 @@ typedef int (*filldir_t)(void *, const char *, int, loff_t, u64, unsigned);
 struct dir_context {
 	const filldir_t actor;
 	loff_t pos;
-};
+} __designated_init;
 
 struct block_device_operations;
 
@@ -1484,7 +1485,7 @@ struct file_operations {
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
 	int (*show_fdinfo)(struct seq_file *m, struct file *f);
-};
+} __designated_init;
 
 struct inode_operations {
 	struct dentry * (*lookup) (struct inode *,struct dentry *, unsigned int);
@@ -1520,7 +1521,7 @@ struct inode_operations {
 			   umode_t create_mode, int *opened);
 	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
 	int (*set_acl)(struct inode *, struct posix_acl *, int);
-} ____cacheline_aligned;
+} __designated_init ____cacheline_aligned;
 
 ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
 			      unsigned long nr_segs, unsigned long fast_segs,
@@ -1561,7 +1562,7 @@ struct super_operations {
 	int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
 	long (*nr_cached_objects)(struct super_block *, int);
 	long (*free_cached_objects)(struct super_block *, long, int);
-};
+} __designated_init;
 
 /*
  * Inode flags - they have no relation to superblock flags now
@@ -1771,7 +1772,7 @@ struct file_system_type {
 	struct lock_class_key i_lock_key;
 	struct lock_class_key i_mutex_key;
 	struct lock_class_key i_mutex_dir_key;
-};
+} __designated_init;
 
 #define MODULE_ALIAS_FS(NAME) MODULE_ALIAS("fs-" NAME)
 
@@ -2018,7 +2019,7 @@ struct filename {
 	const __user char	*uptr;	/* original userland pointer */
 	struct audit_names	*aname;
 	bool			separate; /* should "name" be freed? */
-};
+} __designated_init;
 
 extern long vfs_truncate(struct path *, loff_t);
 extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
@@ -2647,7 +2648,7 @@ static inline ino_t parent_ino(struct dentry *dentry)
 struct simple_transaction_argresp {
 	ssize_t size;
 	char data[0];
-};
+} __designated_init;
 
 #define SIMPLE_TRANSACTION_LIMIT (PAGE_SIZE - sizeof(struct simple_transaction_argresp))
 
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] ftrace: Require designated initialization of structures
  2014-07-31 23:47 [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Josh Triplett
  2014-07-31 23:47 ` [PATCH 2/5] raid: Require designated initialization of structures Josh Triplett
  2014-07-31 23:47 ` [PATCH 3/5] fs: " Josh Triplett
@ 2014-07-31 23:47 ` Josh Triplett
  2014-07-31 23:48 ` [PATCH 5/5] include/linux/interrupt.h: " Josh Triplett
  2014-08-01 20:45 ` [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Andrew Morton
  4 siblings, 0 replies; 9+ messages in thread
From: Josh Triplett @ 2014-07-31 23:47 UTC (permalink / raw)
  To: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Neil Brown,
	Steven Rostedt, linux-api, linux-fsdevel, linux-kbuild,
	linux-kernel, linux-raid, linux-sparse

Mark various ftrace structures with __designated_init.  Fix some ftrace
macros to use designated initializers for those structures.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/ftrace.h       | 4 ++--
 include/linux/ftrace_event.h | 4 ++--
 include/linux/syscalls.h     | 8 ++------
 include/trace/ftrace.h       | 8 ++------
 kernel/trace/trace_export.c  | 4 +---
 5 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 404a686..cb2d023 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -260,7 +260,7 @@ struct ftrace_func_command {
 	int			(*func)(struct ftrace_hash *hash,
 					char *func, char *cmd,
 					char *params, int enable);
-};
+} __designated_init;
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
@@ -283,7 +283,7 @@ struct ftrace_probe_ops {
 					 unsigned long ip,
 					 struct ftrace_probe_ops *ops,
 					 void *data);
-};
+} __designated_init;
 
 extern int
 register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index cff3106..25af313 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -198,7 +198,7 @@ struct ftrace_event_class {
 	struct list_head	*(*get_fields)(struct ftrace_event_call *);
 	struct list_head	fields;
 	int			(*raw_init)(struct ftrace_event_call *);
-};
+} __designated_init;
 
 extern int ftrace_event_reg(struct ftrace_event_call *event,
 			    enum trace_reg type, void *data);
@@ -293,7 +293,7 @@ struct ftrace_event_call {
 	int	(*perf_perm)(struct ftrace_event_call *,
 			     struct perf_event *);
 #endif
-};
+} __designated_init;
 
 static inline const char *
 ftrace_event_name(struct ftrace_event_call *call)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index b0881a0..3002648 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -120,9 +120,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	static struct ftrace_event_call __used				\
 	  event_enter_##sname = {					\
 		.class			= &event_class_syscall_enter,	\
-		{							\
-			.name                   = "sys_enter"#sname,	\
-		},							\
+		.name                   = "sys_enter"#sname,		\
 		.event.funcs            = &enter_syscall_print_funcs,	\
 		.data			= (void *)&__syscall_meta_##sname,\
 		.flags                  = TRACE_EVENT_FL_CAP_ANY,	\
@@ -136,9 +134,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	static struct ftrace_event_call __used				\
 	  event_exit_##sname = {					\
 		.class			= &event_class_syscall_exit,	\
-		{							\
-			.name                   = "sys_exit"#sname,	\
-		},							\
+		.name                   = "sys_exit"#sname,		\
 		.event.funcs		= &exit_syscall_print_funcs,	\
 		.data			= (void *)&__syscall_meta_##sname,\
 		.flags                  = TRACE_EVENT_FL_CAP_ANY,	\
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 26b4f2e..095aaca 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -699,9 +699,7 @@ static struct ftrace_event_class __used __refdata event_class_##call = { \
 									\
 static struct ftrace_event_call __used event_##call = {			\
 	.class			= &event_class_##template,		\
-	{								\
-		.tp			= &__tracepoint_##call,		\
-	},								\
+	.tp			= &__tracepoint_##call,			\
 	.event.funcs		= &ftrace_event_type_funcs_##template,	\
 	.print_fmt		= print_fmt_##template,			\
 	.flags			= TRACE_EVENT_FL_TRACEPOINT,		\
@@ -716,9 +714,7 @@ static const char print_fmt_##call[] = print;				\
 									\
 static struct ftrace_event_call __used event_##call = {			\
 	.class			= &event_class_##template,		\
-	{								\
-		.tp			= &__tracepoint_##call,		\
-	},								\
+	.tp			= &__tracepoint_##call,			\
 	.event.funcs		= &ftrace_event_type_funcs_##call,	\
 	.print_fmt		= print_fmt_##call,			\
 	.flags			= TRACE_EVENT_FL_TRACEPOINT,		\
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index d4ddde2..40f472f 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -174,9 +174,7 @@ struct ftrace_event_class __refdata event_class_ftrace_##call = {	\
 									\
 struct ftrace_event_call __used event_##call = {			\
 	.class			= &event_class_ftrace_##call,		\
-	{								\
-		.name			= #call,			\
-	},								\
+	.name			= #call,				\
 	.event.type		= etype,				\
 	.print_fmt		= print,				\
 	.flags			= TRACE_EVENT_FL_IGNORE_ENABLE | TRACE_EVENT_FL_USE_CALL_FILTER, \
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] include/linux/interrupt.h: Require designated initialization of structures
  2014-07-31 23:47 [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Josh Triplett
                   ` (2 preceding siblings ...)
  2014-07-31 23:47 ` [PATCH 4/5] ftrace: " Josh Triplett
@ 2014-07-31 23:48 ` Josh Triplett
  2014-08-01 20:45 ` [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Andrew Morton
  4 siblings, 0 replies; 9+ messages in thread
From: Josh Triplett @ 2014-07-31 23:48 UTC (permalink / raw)
  To: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Neil Brown,
	Steven Rostedt, linux-api, linux-fsdevel, linux-kbuild,
	linux-kernel, linux-raid, linux-sparse

Fix the corresponding tasklet initialization macros to use designated
initializers, which simplifies those initializers using the default
initialization of fields to 0.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 include/linux/interrupt.h | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 698ad05..559ef98 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -217,7 +217,7 @@ struct irq_affinity_notify {
 	struct work_struct work;
 	void (*notify)(struct irq_affinity_notify *, const cpumask_t *mask);
 	void (*release)(struct kref *ref);
-};
+} __designated_init;
 
 #if defined(CONFIG_SMP)
 
@@ -419,7 +419,7 @@ extern const char * const softirq_to_name[NR_SOFTIRQS];
 struct softirq_action
 {
 	void	(*action)(struct softirq_action *);
-};
+} __designated_init;
 
 asmlinkage void do_softirq(void);
 asmlinkage void __do_softirq(void);
@@ -474,14 +474,21 @@ struct tasklet_struct
 	atomic_t count;
 	void (*func)(unsigned long);
 	unsigned long data;
-};
-
-#define DECLARE_TASKLET(name, func, data) \
-struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(0), func, data }
+} __designated_init;
 
-#define DECLARE_TASKLET_DISABLED(name, func, data) \
-struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(1), func, data }
+#define DECLARE_TASKLET(name, tsfunc, tsdata) \
+struct tasklet_struct name = {		\
+	.count = ATOMIC_INIT(0),	\
+	.func = tsfunc,			\
+	.data = tsdata,			\
+}
 
+#define DECLARE_TASKLET_DISABLED(name, tsfunc, tsdata) \
+struct tasklet_struct name = {		\
+	.count = ATOMIC_INIT(1),	\
+	.func = tsfunc,			\
+	.data = tsdata,			\
+}
 
 enum
 {
@@ -576,7 +583,7 @@ struct tasklet_hrtimer {
 	struct hrtimer		timer;
 	struct tasklet_struct	tasklet;
 	enum hrtimer_restart	(*function)(struct hrtimer *);
-};
+} __designated_init;
 
 extern void
 tasklet_hrtimer_init(struct tasklet_hrtimer *ttimer,
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] raid: Require designated initialization of structures
  2014-07-31 23:47 ` [PATCH 2/5] raid: Require designated initialization of structures Josh Triplett
@ 2014-08-01  1:10   ` NeilBrown
  2014-08-01  1:30     ` Josh Triplett
  0 siblings, 1 reply; 9+ messages in thread
From: NeilBrown @ 2014-08-01  1:10 UTC (permalink / raw)
  To: Josh Triplett
  Cc: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Steven Rostedt,
	linux-api, linux-fsdevel, linux-kbuild, linux-kernel, linux-raid,
	linux-sparse

[-- Attachment #1: Type: text/plain, Size: 9582 bytes --]

On Thu, 31 Jul 2014 16:47:35 -0700 Josh Triplett <josh@joshtriplett.org>
wrote:

> Mark raid6_calls and other structures containing function pointers with
> __designated_init.  Fix implementations in lib/raid6/ to use designated
> initializers; this also simplifies those initializers using the default
> initialization of fields to 0.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Looks like an excellent idea!
Feel free to forward this upstream on my behalf, or remind me once the first
patch is in -next, and I'll take this one myself - whichever you prefer.

 Acked-by: NeilBrown <neilb@suse.de>

Thanks,
NeilBrown

> ---
>  include/linux/raid/pq.h    |  4 ++--
>  include/linux/raid/xor.h   |  2 +-
>  include/linux/raid_class.h |  2 +-
>  lib/raid6/altivec.uc       |  7 +++----
>  lib/raid6/avx2.c           | 24 ++++++++++++------------
>  lib/raid6/int.uc           |  6 ++----
>  lib/raid6/mmx.c            | 14 ++++++--------
>  lib/raid6/neon.c           |  7 +++----
>  lib/raid6/sse1.c           | 16 ++++++++--------
>  lib/raid6/sse2.c           | 24 ++++++++++++------------
>  lib/raid6/tilegx.uc        |  6 ++----
>  11 files changed, 52 insertions(+), 60 deletions(-)
> 
> diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h
> index 73069cb..2147bff 100644
> --- a/include/linux/raid/pq.h
> +++ b/include/linux/raid/pq.h
> @@ -75,7 +75,7 @@ struct raid6_calls {
>  	int  (*valid)(void);	/* Returns 1 if this routine set is usable */
>  	const char *name;	/* Name of this routine set */
>  	int prefer;		/* Has special performance attribute */
> -};
> +} __designated_init;
>  
>  /* Selected algorithm */
>  extern struct raid6_calls raid6_call;
> @@ -109,7 +109,7 @@ struct raid6_recov_calls {
>  	int  (*valid)(void);
>  	const char *name;
>  	int priority;
> -};
> +} __designated_init;
>  
>  extern const struct raid6_recov_calls raid6_recov_intx1;
>  extern const struct raid6_recov_calls raid6_recov_ssse3;
> diff --git a/include/linux/raid/xor.h b/include/linux/raid/xor.h
> index 5a21095..c7df59f 100644
> --- a/include/linux/raid/xor.h
> +++ b/include/linux/raid/xor.h
> @@ -17,6 +17,6 @@ struct xor_block_template {
>  		     unsigned long *, unsigned long *);
>  	void (*do_5)(unsigned long, unsigned long *, unsigned long *,
>  		     unsigned long *, unsigned long *, unsigned long *);
> -};
> +} __designated_init;
>  
>  #endif
> diff --git a/include/linux/raid_class.h b/include/linux/raid_class.h
> index 31e1ff6..603af94 100644
> --- a/include/linux/raid_class.h
> +++ b/include/linux/raid_class.h
> @@ -16,7 +16,7 @@ struct raid_function_template {
>  	int (*is_raid)(struct device *);
>  	void (*get_resync)(struct device *);
>  	void (*get_state)(struct device *);
> -};
> +} __designated_init;
>  
>  enum raid_state {
>  	RAID_STATE_UNKNOWN = 0,
> diff --git a/lib/raid6/altivec.uc b/lib/raid6/altivec.uc
> index 7cc12b5..4ff138c 100644
> --- a/lib/raid6/altivec.uc
> +++ b/lib/raid6/altivec.uc
> @@ -118,10 +118,9 @@ int raid6_have_altivec(void)
>  #endif
>  
>  const struct raid6_calls raid6_altivec$# = {
> -	raid6_altivec$#_gen_syndrome,
> -	raid6_have_altivec,
> -	"altivecx$#",
> -	0
> +	.gen_syndrome = raid6_altivec$#_gen_syndrome,
> +	.valid = raid6_have_altivec,
> +	.name = "altivecx$#",
>  };
>  
>  #endif /* CONFIG_ALTIVEC */
> diff --git a/lib/raid6/avx2.c b/lib/raid6/avx2.c
> index bc3b1dd..e56fa06 100644
> --- a/lib/raid6/avx2.c
> +++ b/lib/raid6/avx2.c
> @@ -88,10 +88,10 @@ static void raid6_avx21_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_avx2x1 = {
> -	raid6_avx21_gen_syndrome,
> -	raid6_have_avx2,
> -	"avx2x1",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_avx21_gen_syndrome,
> +	.valid = raid6_have_avx2,
> +	.name = "avx2x1",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  /*
> @@ -149,10 +149,10 @@ static void raid6_avx22_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_avx2x2 = {
> -	raid6_avx22_gen_syndrome,
> -	raid6_have_avx2,
> -	"avx2x2",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_avx22_gen_syndrome,
> +	.valid = raid6_have_avx2,
> +	.name = "avx2x2",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  #ifdef CONFIG_X86_64
> @@ -241,10 +241,10 @@ static void raid6_avx24_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_avx2x4 = {
> -	raid6_avx24_gen_syndrome,
> -	raid6_have_avx2,
> -	"avx2x4",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_avx24_gen_syndrome,
> +	.valid = raid6_have_avx2,
> +	.name = "avx2x4",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  #endif
>  
> diff --git a/lib/raid6/int.uc b/lib/raid6/int.uc
> index 5b50f8d..35ad01a 100644
> --- a/lib/raid6/int.uc
> +++ b/lib/raid6/int.uc
> @@ -108,10 +108,8 @@ static void raid6_int$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_intx$# = {
> -	raid6_int$#_gen_syndrome,
> -	NULL,		/* always valid */
> -	"int" NSTRING "x$#",
> -	0
> +	.gen_syndrome = raid6_int$#_gen_syndrome,
> +	.name = "int" NSTRING "x$#",
>  };
>  
>  #endif
> diff --git a/lib/raid6/mmx.c b/lib/raid6/mmx.c
> index 590c71c..cdd7d02 100644
> --- a/lib/raid6/mmx.c
> +++ b/lib/raid6/mmx.c
> @@ -75,10 +75,9 @@ static void raid6_mmx1_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_mmxx1 = {
> -	raid6_mmx1_gen_syndrome,
> -	raid6_have_mmx,
> -	"mmxx1",
> -	0
> +	.gen_syndrome = raid6_mmx1_gen_syndrome,
> +	.valid = raid6_have_mmx,
> +	.name = "mmxx1",
>  };
>  
>  /*
> @@ -133,10 +132,9 @@ static void raid6_mmx2_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_mmxx2 = {
> -	raid6_mmx2_gen_syndrome,
> -	raid6_have_mmx,
> -	"mmxx2",
> -	0
> +	.gen_syndrome = raid6_mmx2_gen_syndrome,
> +	.valid = raid6_have_mmx,
> +	.name = "mmxx2",
>  };
>  
>  #endif
> diff --git a/lib/raid6/neon.c b/lib/raid6/neon.c
> index 36ad470..99100dd 100644
> --- a/lib/raid6/neon.c
> +++ b/lib/raid6/neon.c
> @@ -41,10 +41,9 @@
>  		kernel_neon_end();					\
>  	}								\
>  	struct raid6_calls const raid6_neonx ## _n = {			\
> -		raid6_neon ## _n ## _gen_syndrome,			\
> -		raid6_have_neon,					\
> -		"neonx" #_n,						\
> -		0							\
> +		.gen_syndrome = raid6_neon ## _n ## _gen_syndrome,	\
> +		.valid = raid6_have_neon,				\
> +		.name = "neonx" #_n,					\
>  	}
>  
>  static int raid6_have_neon(void)
> diff --git a/lib/raid6/sse1.c b/lib/raid6/sse1.c
> index f762971..a9de46e 100644
> --- a/lib/raid6/sse1.c
> +++ b/lib/raid6/sse1.c
> @@ -91,10 +91,10 @@ static void raid6_sse11_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_sse1x1 = {
> -	raid6_sse11_gen_syndrome,
> -	raid6_have_sse1_or_mmxext,
> -	"sse1x1",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_sse11_gen_syndrome,
> +	.valid = raid6_have_sse1_or_mmxext,
> +	.name = "sse1x1",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  /*
> @@ -153,10 +153,10 @@ static void raid6_sse12_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_sse1x2 = {
> -	raid6_sse12_gen_syndrome,
> -	raid6_have_sse1_or_mmxext,
> -	"sse1x2",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_sse12_gen_syndrome,
> +	.valid = raid6_have_sse1_or_mmxext,
> +	.name = "sse1x2",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  #endif
> diff --git a/lib/raid6/sse2.c b/lib/raid6/sse2.c
> index 85b82c8..cd262518aa 100644
> --- a/lib/raid6/sse2.c
> +++ b/lib/raid6/sse2.c
> @@ -89,10 +89,10 @@ static void raid6_sse21_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_sse2x1 = {
> -	raid6_sse21_gen_syndrome,
> -	raid6_have_sse2,
> -	"sse2x1",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_sse21_gen_syndrome,
> +	.valid = raid6_have_sse2,
> +	.name = "sse2x1",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  /*
> @@ -151,10 +151,10 @@ static void raid6_sse22_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_sse2x2 = {
> -	raid6_sse22_gen_syndrome,
> -	raid6_have_sse2,
> -	"sse2x2",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_sse22_gen_syndrome,
> +	.valid = raid6_have_sse2,
> +	.name = "sse2x2",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  #ifdef CONFIG_X86_64
> @@ -249,10 +249,10 @@ static void raid6_sse24_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_sse2x4 = {
> -	raid6_sse24_gen_syndrome,
> -	raid6_have_sse2,
> -	"sse2x4",
> -	1			/* Has cache hints */
> +	.gen_syndrome = raid6_sse24_gen_syndrome,
> +	.valid = raid6_have_sse2,
> +	.name = "sse2x4",
> +	.prefer = 1,		/* Has cache hints */
>  };
>  
>  #endif /* CONFIG_X86_64 */
> diff --git a/lib/raid6/tilegx.uc b/lib/raid6/tilegx.uc
> index e7c2945..3077722 100644
> --- a/lib/raid6/tilegx.uc
> +++ b/lib/raid6/tilegx.uc
> @@ -79,8 +79,6 @@ void raid6_tilegx$#_gen_syndrome(int disks, size_t bytes, void **ptrs)
>  }
>  
>  const struct raid6_calls raid6_tilegx$# = {
> -	raid6_tilegx$#_gen_syndrome,
> -	NULL,
> -	"tilegx$#",
> -	0
> +	.gen_syndrome = raid6_tilegx$#_gen_syndrome,
> +	.name = "tilegx$#",
>  };


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] raid: Require designated initialization of structures
  2014-08-01  1:10   ` NeilBrown
@ 2014-08-01  1:30     ` Josh Triplett
  0 siblings, 0 replies; 9+ messages in thread
From: Josh Triplett @ 2014-08-01  1:30 UTC (permalink / raw)
  To: NeilBrown
  Cc: akpm, J. Bruce Fields, Alexander Viro, Christopher Li,
	Ingo Molnar, Jeff Layton, Michal Marek, Steven Rostedt,
	linux-api, linux-fsdevel, linux-kbuild, linux-kernel, linux-raid,
	linux-sparse

On Fri, Aug 01, 2014 at 11:10:55AM +1000, NeilBrown wrote:
> On Thu, 31 Jul 2014 16:47:35 -0700 Josh Triplett <josh@joshtriplett.org>
> wrote:
> 
> > Mark raid6_calls and other structures containing function pointers with
> > __designated_init.  Fix implementations in lib/raid6/ to use designated
> > initializers; this also simplifies those initializers using the default
> > initialization of fields to 0.
> > 
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> 
> Looks like an excellent idea!
> Feel free to forward this upstream on my behalf, or remind me once the first
> patch is in -next, and I'll take this one myself - whichever you prefer.
> 
>  Acked-by: NeilBrown <neilb@suse.de>

Thanks!  Ideally, I'd like to see the whole series go in through one
tree, which is why I CCed Andrew.  I can easily produce several dozen
more patches like these, but I just included enough examples to motivate
patch 1, and I can send more in any order once that one goes in.

- Josh Triplett

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init))
  2014-07-31 23:47 [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Josh Triplett
                   ` (3 preceding siblings ...)
  2014-07-31 23:48 ` [PATCH 5/5] include/linux/interrupt.h: " Josh Triplett
@ 2014-08-01 20:45 ` Andrew Morton
  2014-08-01 21:36   ` josh
  4 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2014-08-01 20:45 UTC (permalink / raw)
  To: Josh Triplett
  Cc: J. Bruce Fields, Alexander Viro, Christopher Li, Ingo Molnar,
	Jeff Layton, Michal Marek, Neil Brown, Steven Rostedt, linux-api,
	linux-fsdevel, linux-kbuild, linux-kernel, linux-raid,
	linux-sparse

On Thu, 31 Jul 2014 16:47:23 -0700 Josh Triplett <josh@joshtriplett.org> wrote:

> GCC 4.10 and newer, and Sparse, supports
> __attribute__((designated_init)), which marks a structure as requiring
> a designated initializer rather than a positional one.  This helps
> reduce churn and errors when used with _ops structures and similar
> structures designed for future extension.
> 
> Add a wrapper __designated_init, which turns into
> __attribute__((designated_init)) for Sparse or sufficiently new GCC.
> Enable the corresponding warning as an error.
> 
> The following semantic patch can help mark structures containing
> function pointers as requiring designated initializers:
> 
> @@
> identifier I, f;
> type T;
> @@
> 
> struct I {
> 	...
> 	T (*f)(...);
> 	...
> }
> + __designated_init

hm, dunno about this.

I think that the kernel should always use designated initializers
everywhere.  Perhaps there are a few special cases where positional
initializers provide a superior result but I'm not sure where those
might be.

In which case what we should do is to teach sparse to warn about
positional initializers then go fix them all up (lol).  After that
process is complete, this __designated_init tag would be just noise.

To support this perhaps a sparse tag would be needed which says
"positional initializers are OK here".  This way we're adding the
annotation to the exceptional cases, not to the common cases.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init))
  2014-08-01 20:45 ` [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Andrew Morton
@ 2014-08-01 21:36   ` josh
  0 siblings, 0 replies; 9+ messages in thread
From: josh @ 2014-08-01 21:36 UTC (permalink / raw)
  To: Andrew Morton
  Cc: J. Bruce Fields, Alexander Viro, Christopher Li, Ingo Molnar,
	Jeff Layton, Michal Marek, Neil Brown, Steven Rostedt, linux-api,
	linux-fsdevel, linux-kbuild, linux-kernel, linux-raid,
	linux-sparse

On Fri, Aug 01, 2014 at 01:45:29PM -0700, Andrew Morton wrote:
> On Thu, 31 Jul 2014 16:47:23 -0700 Josh Triplett <josh@joshtriplett.org> wrote:
> 
> > GCC 4.10 and newer, and Sparse, supports
> > __attribute__((designated_init)), which marks a structure as requiring
> > a designated initializer rather than a positional one.  This helps
> > reduce churn and errors when used with _ops structures and similar
> > structures designed for future extension.
> > 
> > Add a wrapper __designated_init, which turns into
> > __attribute__((designated_init)) for Sparse or sufficiently new GCC.
> > Enable the corresponding warning as an error.
> > 
> > The following semantic patch can help mark structures containing
> > function pointers as requiring designated initializers:
> > 
> > @@
> > identifier I, f;
> > type T;
> > @@
> > 
> > struct I {
> > 	...
> > 	T (*f)(...);
> > 	...
> > }
> > + __designated_init
> 
> hm, dunno about this.
> 
> I think that the kernel should always use designated initializers
> everywhere.  Perhaps there are a few special cases where positional
> initializers provide a superior result but I'm not sure where those
> might be.
> 
> In which case what we should do is to teach sparse to warn about
> positional initializers then go fix them all up (lol).  After that
> process is complete, this __designated_init tag would be just noise.
> 
> To support this perhaps a sparse tag would be needed which says
> "positional initializers are OK here".  This way we're adding the
> annotation to the exceptional cases, not to the common cases.

Before submitting this series, I actually used coccinelle to
automatically add __designated_init to nearly a thousand structures
(just in include/linux/ alone), and did a build with that.  Even just
adding it to structures with function pointers, some cases produce a
*huge* number of warnings.  I think it might make sense to work our way
through those warnings, but "go fix them all up" would be a gargantuan
effort.  (For an idea of scale: there were more warnings about
positional initialization than everything other warning combined, by a
substantial factor, and that's just from changing a few hundred
structures.)

I'm not at all convinced that we want to universally enforce designated
initializers *yet*.  They make sense for _ops structures and other
structures that contain function pointers (which is a small subset), but
for many other structures they just add a large amount of noise to
initialization.  If we could say "automatically add __designated_init to
structures matching these criteria", that could work, but "all
structures" not so much.  (For instance, a structure with a single
field, or two fields of incompatible types with an intuitive ordering,
doesn't gain much value from designated initialization.)

Now, some cases may make sense to fix despite the large volume.  For
instance, many users of dmi_system_id initialize it positionally, and
shouldn't; I've already written patches for quite a few of them, with
many more to go.  But, for instance, obs_kernel_param seems much less
worthwhile to fix, because we shouldn't add any new instances of it, and
we likely won't ever extend it.  The value in fixing this issue mostly
comes not with the current code, but with future changes to the
structure.  (And, as always happens with this kind of change, we'll end
up with a few holdouts who don't want to change their code, which will
stop us from eliminating the warning completely.)

In the course of introducing this change, we'll end up fixing a large
number of positional init warnings.  If in the course of doing so, it
starts making sense to enforce it everywhere, it seems easy enough to
sed away all the __designated_init tags.  But in terms of noise, the
actual additions of __designated_init will pale in comparison to the
patches fixing positional initializers.

Finally, by migrating over to this incrementally, structure by
structure, we can safely make the warning an error, which we could not
do if we applied it to every struct immediately.

- Josh Triplett

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-08-01 21:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-31 23:47 [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Josh Triplett
2014-07-31 23:47 ` [PATCH 2/5] raid: Require designated initialization of structures Josh Triplett
2014-08-01  1:10   ` NeilBrown
2014-08-01  1:30     ` Josh Triplett
2014-07-31 23:47 ` [PATCH 3/5] fs: " Josh Triplett
2014-07-31 23:47 ` [PATCH 4/5] ftrace: " Josh Triplett
2014-07-31 23:48 ` [PATCH 5/5] include/linux/interrupt.h: " Josh Triplett
2014-08-01 20:45 ` [PATCH 1/5] Add __designated_init, wrapping __attribute__((designated_init)) Andrew Morton
2014-08-01 21:36   ` josh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).