2024-06-13 19:24:29

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH] linux++: delete some forward declarations

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 <[email protected]>
---

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)
{


2024-06-13 19:34:17

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] linux++: delete some forward declarations

On Thu, 13 Jun 2024 22:22:18 +0300
Alexey Dobriyan <[email protected]> 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 <[email protected]>
> ---
>
> 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)
> {


2024-06-13 20:10:22

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] linux++: delete some forward declarations

On Thu, 13 Jun 2024 13:04:20 -0700
Andrew Morton <[email protected]> wrote:

> On Thu, 13 Jun 2024 15:34:02 -0400 Steven Rostedt <[email protected]> wrote:
>
> > On Thu, 13 Jun 2024 22:22:18 +0300
> > Alexey Dobriyan <[email protected]> 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

2024-06-13 20:27:57

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] linux++: delete some forward declarations

On Thu, 13 Jun 2024 15:34:02 -0400 Steven Rostedt <[email protected]> wrote:

> On Thu, 13 Jun 2024 22:22:18 +0300
> Alexey Dobriyan <[email protected]> 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.

2024-06-13 21:21:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] linux++: delete some forward declarations

On Thu, 13 Jun 2024 16:10:12 -0400 Steven Rostedt <[email protected]> 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.

2024-06-13 21:28:48

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] linux++: delete some forward declarations

On Thu, 13 Jun 2024 14:21:10 -0700
Andrew Morton <[email protected]> wrote:

> On Thu, 13 Jun 2024 16:10:12 -0400 Steven Rostedt <[email protected]> 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

2024-06-14 06:29:48

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] linux++: delete some forward declarations

On Thu, Jun 13, 2024 at 04:10:12PM -0400, Steven Rostedt wrote:
> On Thu, 13 Jun 2024 13:04:20 -0700
> Andrew Morton <[email protected]> wrote:
>
> > On Thu, 13 Jun 2024 15:34:02 -0400 Steven Rostedt <[email protected]> wrote:
> >
> > > On Thu, 13 Jun 2024 22:22:18 +0300
> > > Alexey Dobriyan <[email protected]> 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.