2020-08-03 12:37:33

by Jules Irenge

[permalink] [raw]
Subject: [RESEND PATCH 0/2] cleanups

I am proposing these 2 patches, one of which is recommendation of Joe Perches,
namely the removing of unnecessary globals and change them to static.
I am currently learning the core kernel the hard way.
I will appreciate any feedback negative or positive.
Thanks

Jules Irenge (2):
audit: change unnecessary globals into statics
audit: uninitialize variable audit_sig_sid

kernel/audit.c | 6 +++---
kernel/audit.h | 4 ----
2 files changed, 3 insertions(+), 7 deletions(-)

--
2.26.2


2020-08-03 12:37:40

by Jules Irenge

[permalink] [raw]
Subject: [RESEND PATCH 1/2] audit: change unnecessary globals into statics

Variables sig_pid, audit_sig_uid and audit_sig_sid
are only used in the audit.c file across the kernel
Hence it appears no reason for declaring them as globals
This patch removes their global declarations from the .h file
and change them into static in the .c file.

Signed-off-by: Jules Irenge <[email protected]>
---
kernel/audit.c | 6 +++---
kernel/audit.h | 4 ----
2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index b2301bdc9773..afd7827cf6e8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -123,9 +123,9 @@ static u32 audit_backlog_limit = 64;
static u32 audit_backlog_wait_time = AUDIT_BACKLOG_WAIT_TIME;

/* The identity of the user shutting down the audit system. */
-kuid_t audit_sig_uid = INVALID_UID;
-pid_t audit_sig_pid = -1;
-u32 audit_sig_sid = 0;
+static kuid_t audit_sig_uid = INVALID_UID;
+static pid_t audit_sig_pid = -1;
+static u32 audit_sig_sid = 0;

/* Records can be lost in several ways:
0) [suppressed in audit_alloc]
diff --git a/kernel/audit.h b/kernel/audit.h
index ddc22878433d..3b9c0945225a 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -327,10 +327,6 @@ static inline int audit_signal_info_syscall(struct task_struct *t)

extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);

-extern pid_t audit_sig_pid;
-extern kuid_t audit_sig_uid;
-extern u32 audit_sig_sid;
-
extern int audit_filter(int msgtype, unsigned int listtype);

extern void audit_ctl_lock(void);
--
2.26.2

2020-08-03 12:37:45

by Jules Irenge

[permalink] [raw]
Subject: [RESEND PATCH 2/2] audit: uninitialize variable audit_sig_sid

Checkpatch tool reports

"ERROR: do not initialise globals/statics to 0"

To fix this, audit_sig_sid is uninitialized
As this is stored in the .bss section,
the compiler can initialize the variable automatically.

Signed-off-by: Jules Irenge <[email protected]>
---
kernel/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index afd7827cf6e8..1c74d1d788b6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -125,7 +125,7 @@ static u32 audit_backlog_wait_time = AUDIT_BACKLOG_WAIT_TIME;
/* The identity of the user shutting down the audit system. */
static kuid_t audit_sig_uid = INVALID_UID;
static pid_t audit_sig_pid = -1;
-static u32 audit_sig_sid = 0;
+static u32 audit_sig_sid;

/* Records can be lost in several ways:
0) [suppressed in audit_alloc]
--
2.26.2

2020-08-06 18:35:46

by Paul Moore

[permalink] [raw]
Subject: Re: [RESEND PATCH 1/2] audit: change unnecessary globals into statics

On Mon, Aug 3, 2020 at 8:35 AM Jules Irenge <[email protected]> wrote:
>
> Variables sig_pid, audit_sig_uid and audit_sig_sid
> are only used in the audit.c file across the kernel
> Hence it appears no reason for declaring them as globals
> This patch removes their global declarations from the .h file
> and change them into static in the .c file.
>
> Signed-off-by: Jules Irenge <[email protected]>
> ---
> kernel/audit.c | 6 +++---
> kernel/audit.h | 4 ----
> 2 files changed, 3 insertions(+), 7 deletions(-)

Thanks Jules, this looks reasonable although I'm not going to merge
them into audit/next until after the merge window closes. I'll send
another reply once this has been merged.

--
paul moore
http://www.paul-moore.com

2020-08-06 18:39:43

by Paul Moore

[permalink] [raw]
Subject: Re: [RESEND PATCH 2/2] audit: uninitialize variable audit_sig_sid

On Mon, Aug 3, 2020 at 8:35 AM Jules Irenge <[email protected]> wrote:
>
> Checkpatch tool reports
>
> "ERROR: do not initialise globals/statics to 0"
>
> To fix this, audit_sig_sid is uninitialized
> As this is stored in the .bss section,
> the compiler can initialize the variable automatically.
>
> Signed-off-by: Jules Irenge <[email protected]>
> ---
> kernel/audit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Similar to patch 1/2, this will need to wait until after the merge
window closes.

--
paul moore
http://www.paul-moore.com

2020-08-18 00:40:18

by Paul Moore

[permalink] [raw]
Subject: Re: [RESEND PATCH 1/2] audit: change unnecessary globals into statics

On Thu, Aug 6, 2020 at 2:33 PM Paul Moore <[email protected]> wrote:
>
> On Mon, Aug 3, 2020 at 8:35 AM Jules Irenge <[email protected]> wrote:
> >
> > Variables sig_pid, audit_sig_uid and audit_sig_sid
> > are only used in the audit.c file across the kernel
> > Hence it appears no reason for declaring them as globals
> > This patch removes their global declarations from the .h file
> > and change them into static in the .c file.
> >
> > Signed-off-by: Jules Irenge <[email protected]>
> > ---
> > kernel/audit.c | 6 +++---
> > kernel/audit.h | 4 ----
> > 2 files changed, 3 insertions(+), 7 deletions(-)
>
> Thanks Jules, this looks reasonable although I'm not going to merge
> them into audit/next until after the merge window closes. I'll send
> another reply once this has been merged.

... and I just merged this into audit/next, thanks Jules.

--
paul moore
http://www.paul-moore.com

2020-08-18 00:41:08

by Paul Moore

[permalink] [raw]
Subject: Re: [RESEND PATCH 2/2] audit: uninitialize variable audit_sig_sid

On Thu, Aug 6, 2020 at 2:35 PM Paul Moore <[email protected]> wrote:
>
> On Mon, Aug 3, 2020 at 8:35 AM Jules Irenge <[email protected]> wrote:
> >
> > Checkpatch tool reports
> >
> > "ERROR: do not initialise globals/statics to 0"
> >
> > To fix this, audit_sig_sid is uninitialized
> > As this is stored in the .bss section,
> > the compiler can initialize the variable automatically.
> >
> > Signed-off-by: Jules Irenge <[email protected]>
> > ---
> > kernel/audit.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Similar to patch 1/2, this will need to wait until after the merge
> window closes.

... also merged this patch into audit/next. Thanks again.

--
paul moore
http://www.paul-moore.com