| Message ID | 5ad5556c-7c32-45b7-89cf-f723c9d7332b@p183 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | linux++: delete some forward declarations | expand |
On Thu, 13 Jun 2024 22:22:18 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote: > g++ doesn't like forward enum declarations: > > error: use of enum ‘E’ without previous declaration > 64 | enum E; But we don't care about g++. Do we? I would make that a separate patch. > > Delete those which aren't used. > > Delete some unused/unnecessary forward struct declarations for a change. This is a clean up, but should have a better change log. Just something simple like: Delete unnecessary forward struct declarations. Thanks, -- Steve > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > --- > > fs/ramfs/inode.c | 1 - > include/linux/console.h | 2 -- > include/linux/device.h | 3 --- > include/linux/ftrace.h | 4 ---- > include/linux/security.h | 6 ------ > include/linux/signal.h | 2 -- > include/linux/syscalls.h | 7 ------- > include/linux/sysfs.h | 2 -- > mm/internal.h | 4 ---- > mm/shmem.c | 1 - > 10 files changed, 32 deletions(-) > > --- a/fs/ramfs/inode.c > +++ b/fs/ramfs/inode.c > @@ -51,7 +51,6 @@ struct ramfs_fs_info { > > #define RAMFS_DEFAULT_MODE 0755 > > -static const struct super_operations ramfs_ops; > static const struct inode_operations ramfs_dir_inode_operations; > > struct inode *ramfs_get_inode(struct super_block *sb, > --- a/include/linux/console.h > +++ b/include/linux/console.h > @@ -21,10 +21,8 @@ > #include <linux/vesa.h> > > struct vc_data; > -struct console_font_op; > struct console_font; > struct module; > -struct tty_struct; > struct notifier_block; > > enum con_scroll { > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -36,10 +36,7 @@ > struct device; > struct device_private; > struct device_driver; > -struct driver_private; > struct module; > -struct class; > -struct subsys_private; > struct device_node; > struct fwnode_handle; > struct iommu_group; > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -531,8 +531,6 @@ extern const void *ftrace_expected; > > void ftrace_bug(int err, struct dyn_ftrace *rec); > > -struct seq_file; > - > extern int ftrace_text_reserved(const void *start, const void *end); > > struct ftrace_ops *ftrace_ops_trampoline(unsigned long addr); > @@ -1147,8 +1145,6 @@ static inline void unpause_graph_tracing(void) { } > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > > #ifdef CONFIG_TRACING > -enum ftrace_dump_mode; > - > #define MAX_TRACER_SIZE 100 > extern char ftrace_dump_on_oops[]; > extern int ftrace_dump_on_oops_enabled(void); > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -41,7 +41,6 @@ struct rlimit; > struct kernel_siginfo; > struct sembuf; > struct kern_ipc_perm; > -struct audit_context; > struct super_block; > struct inode; > struct dentry; > @@ -59,8 +58,6 @@ struct xfrm_sec_ctx; > struct mm_struct; > struct fs_context; > struct fs_parameter; > -enum fs_value_type; > -struct watch; > struct watch_notification; > struct lsm_ctx; > > @@ -183,8 +180,6 @@ struct sock; > struct sockaddr; > struct socket; > struct flowi_common; > -struct dst_entry; > -struct xfrm_selector; > struct xfrm_policy; > struct xfrm_state; > struct xfrm_user_sec_ctx; > @@ -219,7 +214,6 @@ extern unsigned long dac_mmap_min_addr; > #define LSM_PRLIMIT_WRITE 2 > > /* forward declares to avoid warnings */ > -struct sched_param; > struct request_sock; > > /* bprm->unsafe reasons */ > --- a/include/linux/signal.h > +++ b/include/linux/signal.h > @@ -274,8 +274,6 @@ static inline int valid_signal(unsigned long sig) > return sig <= _NSIG ? 1 : 0; > } > > -struct timespec; > -struct pt_regs; > enum pid_type; > > extern int next_signal(struct sigpending *pending, sigset_t *mask); > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -11,8 +11,6 @@ > > struct __aio_sigset; > struct epoll_event; > -struct iattr; > -struct inode; > struct iocb; > struct io_event; > struct iovec; > @@ -20,14 +18,12 @@ struct __kernel_old_itimerval; > struct kexec_segment; > struct linux_dirent; > struct linux_dirent64; > -struct list_head; > struct mmap_arg_struct; > struct msgbuf; > struct user_msghdr; > struct mmsghdr; > struct msqid_ds; > struct new_utsname; > -struct nfsctl_arg; > struct __old_kernel_stat; > struct oldold_utsname; > struct old_utsname; > @@ -38,7 +34,6 @@ struct rusage; > struct sched_param; > struct sched_attr; > struct sel_arg_struct; > -struct semaphore; > struct sembuf; > struct shmid_ds; > struct sockaddr; > @@ -48,14 +43,12 @@ struct statfs; > struct statfs64; > struct statx; > struct sysinfo; > -struct timespec; > struct __kernel_old_timeval; > struct __kernel_timex; > struct timezone; > struct tms; > struct utimbuf; > struct mq_attr; > -struct compat_stat; > struct old_timeval32; > struct robust_list_head; > struct futex_waitv; > --- a/include/linux/sysfs.h > +++ b/include/linux/sysfs.h > @@ -23,9 +23,7 @@ > #include <linux/atomic.h> > > struct kobject; > -struct module; > struct bin_attribute; > -enum kobj_ns_type; > > struct attribute { > const char *name; > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -1095,10 +1095,6 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone, > /* Flags that allow allocations below the min watermark. */ > #define ALLOC_RESERVES (ALLOC_NON_BLOCK|ALLOC_MIN_RESERVE|ALLOC_HIGHATOMIC|ALLOC_OOM) > > -enum ttu_flags; > -struct tlbflush_unmap_batch; > - > - > /* > * only for MM internal work items which do not depend on > * any allocations or locks which might depend on allocations > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -261,7 +261,6 @@ static const struct inode_operations shmem_dir_inode_operations; > static const struct inode_operations shmem_special_inode_operations; > static const struct vm_operations_struct shmem_vm_ops; > static const struct vm_operations_struct shmem_anon_vm_ops; > -static struct file_system_type shmem_fs_type; > > bool shmem_mapping(struct address_space *mapping) > {
On Thu, 13 Jun 2024 15:34:02 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 13 Jun 2024 22:22:18 +0300 > Alexey Dobriyan <adobriyan@gmail.com> wrote: > > > g++ doesn't like forward enum declarations: > > > > error: use of enum ‘E’ without previous declaration > > 64 | enum E; > > But we don't care about g++. Do we? It appears that g++ is a useful enum declaration detector. I'm curious to know how even the above warning was generated. Does g++ work at all on Linux? > I would make that a separate patch. What are you referring to here? > > > > Delete those which aren't used. > > > > Delete some unused/unnecessary forward struct declarations for a change. > > This is a clean up, but should have a better change log. Just something > simple like: > > Delete unnecessary forward struct declarations. Alexey specializes in cute changelogs. I do have a concern about the patch: has it been tested with all possible Kconfigs? No. There may be some configs in which the forward declaration is required. And... I'm a bit surprised that forward declarations are allowed in C. A billion years ago I used a C compiler which would use 16 bits for an enum if the enumted values would fit in 16 bits. And it would use 32 bits otherwise. So the enumerated values were *required* for the compiler to be able to figure out the sizeof. But it was a billion years ago.
On Thu, 13 Jun 2024 13:04:20 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 13 Jun 2024 15:34:02 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Thu, 13 Jun 2024 22:22:18 +0300 > > Alexey Dobriyan <adobriyan@gmail.com> wrote: > > > > > g++ doesn't like forward enum declarations: > > > > > > error: use of enum ‘E’ without previous declaration > > > 64 | enum E; > > > > But we don't care about g++. Do we? > > It appears that g++ is a useful enum declaration detector. > > I'm curious to know how even the above warning was generated. Does g++ > work at all on Linux? > > > I would make that a separate patch. > > What are you referring to here? The enum change should be separate from the struct changes. > > > > > > > Delete those which aren't used. > > > > > > Delete some unused/unnecessary forward struct declarations for a change. > > > > This is a clean up, but should have a better change log. Just something > > simple like: > > > > Delete unnecessary forward struct declarations. > > Alexey specializes in cute changelogs. eh > > I do have a concern about the patch: has it been tested with all > possible Kconfigs? No. There may be some configs in which the forward > declaration is required. > > And... I'm a bit surprised that forward declarations are allowed in C. > A billion years ago I used a C compiler which would use 16 bits for > an enum if the enumted values would fit in 16 bits. And it would use 32 > bits otherwise. So the enumerated values were *required* for the > compiler to be able to figure out the sizeof. But it was a billion > years ago. Well, I only looked at the one change in ftrace.h which has a "struct seq_file;" that is not used anywhere else in the file, so that one definitely can go. -- Steve
On Thu, 13 Jun 2024 16:10:12 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > And... I'm a bit surprised that forward declarations are allowed in C. > > A billion years ago I used a C compiler which would use 16 bits for > > an enum if the enumted values would fit in 16 bits. And it would use 32 > > bits otherwise. So the enumerated values were *required* for the > > compiler to be able to figure out the sizeof. But it was a billion > > years ago. > > Well, I only looked at the one change in ftrace.h which has a > "struct seq_file;" that is not used anywhere else in the file, so that > one definitely can go. The risk is that something which includes ftrace.h is depending upon the enum declaration.
On Thu, 13 Jun 2024 14:21:10 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 13 Jun 2024 16:10:12 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > > > And... I'm a bit surprised that forward declarations are allowed in C. > > > A billion years ago I used a C compiler which would use 16 bits for > > > an enum if the enumted values would fit in 16 bits. And it would use 32 > > > bits otherwise. So the enumerated values were *required* for the > > > compiler to be able to figure out the sizeof. But it was a billion > > > years ago. > > > > Well, I only looked at the one change in ftrace.h which has a > > "struct seq_file;" that is not used anywhere else in the file, so that > > one definitely can go. > > The risk is that something which includes ftrace.h is depending upon > the enum declaration. You mean forward struct declaration. And if so, good! it needs to be fixed ;-) -- Steve
On Thu, Jun 13, 2024 at 04:10:12PM -0400, Steven Rostedt wrote: > On Thu, 13 Jun 2024 13:04:20 -0700 > Andrew Morton <akpm@linux-foundation.org> wrote: > > > On Thu, 13 Jun 2024 15:34:02 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > On Thu, 13 Jun 2024 22:22:18 +0300 > > > Alexey Dobriyan <adobriyan@gmail.com> wrote: > > > > > > > g++ doesn't like forward enum declarations: > > > > > > > > error: use of enum ‘E’ without previous declaration > > > > 64 | enum E; > > > > > > But we don't care about g++. Do we? > > > > It appears that g++ is a useful enum declaration detector. > > > > I'm curious to know how even the above warning was generated. Does g++ > > work at all on Linux? With out-of-tree patch, yes. What happens is that "enum E;" works in C but doesn't work in C++. The fix (in C++) is to either delete, or change to "enum E:int;". The same applies to const struct S s; const struct S s = {}; First declaration is compile error in C++, sometimes it can be deleted. This patch is some "unused" parts merged together because it doesn't make sense to split this much -- every chunk is independent of each other. > > > I would make that a separate patch. > > > > What are you referring to here? > > The enum change should be separate from the struct changes. > > > > > > > > > > > Delete those which aren't used. > > > > > > > > Delete some unused/unnecessary forward struct declarations for a change. > > > > > > This is a clean up, but should have a better change log. Just something > > > simple like: > > > > > > Delete unnecessary forward struct declarations. > > > > Alexey specializes in cute changelogs. > > eh Steven is right. That's what my literature teacher said in high school. > > I do have a concern about the patch: has it been tested with all > > possible Kconfigs? No. There may be some configs in which the forward > > declaration is required. > > > > And... I'm a bit surprised that forward declarations are allowed in C. > > A billion years ago I used a C compiler which would use 16 bits for > > an enum if the enumted values would fit in 16 bits. And it would use 32 > > bits otherwise. So the enumerated values were *required* for the > > compiler to be able to figure out the sizeof. But it was a billion > > years ago. > > Well, I only looked at the one change in ftrace.h which has a > "struct seq_file;" that is not used anywhere else in the file, so that > one definitely can go. It was tested on arm64 allmodconfig too. OK if this is concern, I could dust off my compile test farm.
--- a/fs/ramfs/inode.c +++ b/fs/ramfs/inode.c @@ -51,7 +51,6 @@ struct ramfs_fs_info { #define RAMFS_DEFAULT_MODE 0755 -static const struct super_operations ramfs_ops; static const struct inode_operations ramfs_dir_inode_operations; struct inode *ramfs_get_inode(struct super_block *sb, --- a/include/linux/console.h +++ b/include/linux/console.h @@ -21,10 +21,8 @@ #include <linux/vesa.h> struct vc_data; -struct console_font_op; struct console_font; struct module; -struct tty_struct; struct notifier_block; enum con_scroll { --- a/include/linux/device.h +++ b/include/linux/device.h @@ -36,10 +36,7 @@ struct device; struct device_private; struct device_driver; -struct driver_private; struct module; -struct class; -struct subsys_private; struct device_node; struct fwnode_handle; struct iommu_group; --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -531,8 +531,6 @@ extern const void *ftrace_expected; void ftrace_bug(int err, struct dyn_ftrace *rec); -struct seq_file; - extern int ftrace_text_reserved(const void *start, const void *end); struct ftrace_ops *ftrace_ops_trampoline(unsigned long addr); @@ -1147,8 +1145,6 @@ static inline void unpause_graph_tracing(void) { } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ #ifdef CONFIG_TRACING -enum ftrace_dump_mode; - #define MAX_TRACER_SIZE 100 extern char ftrace_dump_on_oops[]; extern int ftrace_dump_on_oops_enabled(void); --- a/include/linux/security.h +++ b/include/linux/security.h @@ -41,7 +41,6 @@ struct rlimit; struct kernel_siginfo; struct sembuf; struct kern_ipc_perm; -struct audit_context; struct super_block; struct inode; struct dentry; @@ -59,8 +58,6 @@ struct xfrm_sec_ctx; struct mm_struct; struct fs_context; struct fs_parameter; -enum fs_value_type; -struct watch; struct watch_notification; struct lsm_ctx; @@ -183,8 +180,6 @@ struct sock; struct sockaddr; struct socket; struct flowi_common; -struct dst_entry; -struct xfrm_selector; struct xfrm_policy; struct xfrm_state; struct xfrm_user_sec_ctx; @@ -219,7 +214,6 @@ extern unsigned long dac_mmap_min_addr; #define LSM_PRLIMIT_WRITE 2 /* forward declares to avoid warnings */ -struct sched_param; struct request_sock; /* bprm->unsafe reasons */ --- a/include/linux/signal.h +++ b/include/linux/signal.h @@ -274,8 +274,6 @@ static inline int valid_signal(unsigned long sig) return sig <= _NSIG ? 1 : 0; } -struct timespec; -struct pt_regs; enum pid_type; extern int next_signal(struct sigpending *pending, sigset_t *mask); --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -11,8 +11,6 @@ struct __aio_sigset; struct epoll_event; -struct iattr; -struct inode; struct iocb; struct io_event; struct iovec; @@ -20,14 +18,12 @@ struct __kernel_old_itimerval; struct kexec_segment; struct linux_dirent; struct linux_dirent64; -struct list_head; struct mmap_arg_struct; struct msgbuf; struct user_msghdr; struct mmsghdr; struct msqid_ds; struct new_utsname; -struct nfsctl_arg; struct __old_kernel_stat; struct oldold_utsname; struct old_utsname; @@ -38,7 +34,6 @@ struct rusage; struct sched_param; struct sched_attr; struct sel_arg_struct; -struct semaphore; struct sembuf; struct shmid_ds; struct sockaddr; @@ -48,14 +43,12 @@ struct statfs; struct statfs64; struct statx; struct sysinfo; -struct timespec; struct __kernel_old_timeval; struct __kernel_timex; struct timezone; struct tms; struct utimbuf; struct mq_attr; -struct compat_stat; struct old_timeval32; struct robust_list_head; struct futex_waitv; --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -23,9 +23,7 @@ #include <linux/atomic.h> struct kobject; -struct module; struct bin_attribute; -enum kobj_ns_type; struct attribute { const char *name; --- a/mm/internal.h +++ b/mm/internal.h @@ -1095,10 +1095,6 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone, /* Flags that allow allocations below the min watermark. */ #define ALLOC_RESERVES (ALLOC_NON_BLOCK|ALLOC_MIN_RESERVE|ALLOC_HIGHATOMIC|ALLOC_OOM) -enum ttu_flags; -struct tlbflush_unmap_batch; - - /* * only for MM internal work items which do not depend on * any allocations or locks which might depend on allocations --- a/mm/shmem.c +++ b/mm/shmem.c @@ -261,7 +261,6 @@ static const struct inode_operations shmem_dir_inode_operations; static const struct inode_operations shmem_special_inode_operations; static const struct vm_operations_struct shmem_vm_ops; static const struct vm_operations_struct shmem_anon_vm_ops; -static struct file_system_type shmem_fs_type; bool shmem_mapping(struct address_space *mapping) {
g++ doesn't like forward enum declarations: error: use of enum ‘E’ without previous declaration 64 | enum E; Delete those which aren't used. Delete some unused/unnecessary forward struct declarations for a change. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- fs/ramfs/inode.c | 1 - include/linux/console.h | 2 -- include/linux/device.h | 3 --- include/linux/ftrace.h | 4 ---- include/linux/security.h | 6 ------ include/linux/signal.h | 2 -- include/linux/syscalls.h | 7 ------- include/linux/sysfs.h | 2 -- mm/internal.h | 4 ---- mm/shmem.c | 1 - 10 files changed, 32 deletions(-)