* [PATCH v2 0/4] Coccinelle cleanups
[not found] <0/4>
@ 2020-03-30 1:24 ` Jules Irenge
2020-03-30 1:24 ` [PATCH v2 1/4] cpu: Remove Comparison to bool Jules Irenge
` (3 more replies)
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
1 sibling, 4 replies; 21+ messages in thread
From: Jules Irenge @ 2020-03-30 1:24 UTC (permalink / raw)
To: linux-kernel; +Cc: julia.lawall, boqun.feng
This patch series clean up some warnings by the Coccinelle tool.
Jules Irenge (4):
cpu: Remove Comparison to bool
rcu: Replace 1 by true
genirq: Replace 1 by true
locking/rtmutex: Remove Comparison to bool
kernel/cpu.c | 2 +-
kernel/irq/spurious.c | 2 +-
kernel/locking/rtmutex.c | 2 +-
kernel/rcu/tree.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
--
Changes since v2:
- Improve on commit log
- Correct mistake of mixing two different subsystems patches into one.
2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/4] cpu: Remove Comparison to bool
2020-03-30 1:24 ` [PATCH v2 0/4] Coccinelle cleanups Jules Irenge
@ 2020-03-30 1:24 ` Jules Irenge
2020-03-30 1:24 ` [PATCH v2 2/4] rcu: Replace 1 by true Jules Irenge
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-03-30 1:24 UTC (permalink / raw)
To: linux-kernel
Cc: julia.lawall, boqun.feng, Thomas Gleixner, Jiri Kosina,
Peter Zijlstra, Josh Poimboeuf, Paolo Bonzini, Arnd Bergmann,
Eiichi Tsukata, Tyler Hicks
Coccinelle reports a warning inside __cpuhp_state_add_instance_cpuslocked()
WARNING: Comparison to bool
To fix this a comparison to a bool variable to false is removed
and replaced with the operator ! before the variable
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 9c706af713fb..97f8b79ba5f5 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1682,7 +1682,7 @@ int __cpuhp_state_add_instance_cpuslocked(enum cpuhp_state state,
lockdep_assert_cpus_held();
sp = cpuhp_get_step(state);
- if (sp->multi_instance == false)
+ if (!sp->multi_instance)
return -EINVAL;
mutex_lock(&cpuhp_state_mutex);
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/4] rcu: Replace 1 by true
2020-03-30 1:24 ` [PATCH v2 0/4] Coccinelle cleanups Jules Irenge
2020-03-30 1:24 ` [PATCH v2 1/4] cpu: Remove Comparison to bool Jules Irenge
@ 2020-03-30 1:24 ` Jules Irenge
2020-03-30 21:16 ` Paul E. McKenney
2020-03-30 1:24 ` [PATCH v2 3/4] genirq: " Jules Irenge
2020-03-30 1:24 ` [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool Jules Irenge
3 siblings, 1 reply; 21+ messages in thread
From: Jules Irenge @ 2020-03-30 1:24 UTC (permalink / raw)
To: linux-kernel
Cc: julia.lawall, boqun.feng, Paul E. McKenney, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes,
open list:READ-COPY UPDATE (RCU)
Coccinelle reports a warning at use_softirq declaration
WARNING: Assignment of 0/1 to bool variable
The root cause is
use_softirq a variable of bool type is initialised with the integer 1
Replacing 1 with value true solve the issue.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/rcu/tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index d91c9156fab2..c2ffea31eae8 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -100,7 +100,7 @@ static struct rcu_state rcu_state = {
static bool dump_tree;
module_param(dump_tree, bool, 0444);
/* By default, use RCU_SOFTIRQ instead of rcuc kthreads. */
-static bool use_softirq = 1;
+static bool use_softirq = true;
module_param(use_softirq, bool, 0444);
/* Control rcu_node-tree auto-balancing at boot time. */
static bool rcu_fanout_exact;
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/4] genirq: Replace 1 by true
2020-03-30 1:24 ` [PATCH v2 0/4] Coccinelle cleanups Jules Irenge
2020-03-30 1:24 ` [PATCH v2 1/4] cpu: Remove Comparison to bool Jules Irenge
2020-03-30 1:24 ` [PATCH v2 2/4] rcu: Replace 1 by true Jules Irenge
@ 2020-03-30 1:24 ` Jules Irenge
2020-03-30 1:24 ` [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool Jules Irenge
3 siblings, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-03-30 1:24 UTC (permalink / raw)
To: linux-kernel; +Cc: julia.lawall, boqun.feng, Thomas Gleixner
Coccinelle reports a warning at noirqdebug declaration
WARNING: Assignment of 0/1 to bool variable
The root cause is that
noirqdebug, a variable of bool type is initialised with the integer 1
Replacing 1 with value true fixes the issue.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/irq/spurious.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index f865e5f4d382..70ba6d55d02a 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -431,7 +431,7 @@ bool noirqdebug __read_mostly;
int noirqdebug_setup(char *str)
{
- noirqdebug = 1;
+ noirqdebug = true;
printk(KERN_INFO "IRQ lockup detection disabled\n");
return 1;
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool
2020-03-30 1:24 ` [PATCH v2 0/4] Coccinelle cleanups Jules Irenge
` (2 preceding siblings ...)
2020-03-30 1:24 ` [PATCH v2 3/4] genirq: " Jules Irenge
@ 2020-03-30 1:24 ` Jules Irenge
2020-03-30 11:21 ` Peter Zijlstra
3 siblings, 1 reply; 21+ messages in thread
From: Jules Irenge @ 2020-03-30 1:24 UTC (permalink / raw)
To: linux-kernel
Cc: julia.lawall, boqun.feng, Peter Zijlstra, Ingo Molnar, Will Deacon
Coccinelle reports a warning inside __sched rt_mutex_slowunlock()
WARNING: Comparison to bool
To fix this,
a comparison (==) of a bool type function result to value true
together with the value are removed.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/locking/rtmutex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 851bbb10819d..7289e7b26be4 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1378,7 +1378,7 @@ static bool __sched rt_mutex_slowunlock(struct rt_mutex *lock,
*/
while (!rt_mutex_has_waiters(lock)) {
/* Drops lock->wait_lock ! */
- if (unlock_rt_mutex_safe(lock, flags) == true)
+ if (unlock_rt_mutex_safe(lock, flags))
return false;
/* Relock the rtmutex and try again */
raw_spin_lock_irqsave(&lock->wait_lock, flags);
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool
2020-03-30 1:24 ` [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool Jules Irenge
@ 2020-03-30 11:21 ` Peter Zijlstra
2020-04-03 16:17 ` Jules Irenge
0 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2020-03-30 11:21 UTC (permalink / raw)
To: Jules Irenge
Cc: linux-kernel, julia.lawall, boqun.feng, Ingo Molnar, Will Deacon
On Mon, Mar 30, 2020 at 02:24:50AM +0100, Jules Irenge wrote:
> Coccinelle reports a warning inside __sched rt_mutex_slowunlock()
>
> WARNING: Comparison to bool
I don't mind the patch; but WTH is that a WARNING ?!? Superfluous, but
definitely not wrong or even dangerous AFAICT.
> To fix this,
> a comparison (==) of a bool type function result to value true
> together with the value are removed.
>
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> kernel/locking/rtmutex.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index 851bbb10819d..7289e7b26be4 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1378,7 +1378,7 @@ static bool __sched rt_mutex_slowunlock(struct rt_mutex *lock,
> */
> while (!rt_mutex_has_waiters(lock)) {
> /* Drops lock->wait_lock ! */
> - if (unlock_rt_mutex_safe(lock, flags) == true)
> + if (unlock_rt_mutex_safe(lock, flags))
> return false;
> /* Relock the rtmutex and try again */
> raw_spin_lock_irqsave(&lock->wait_lock, flags);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/4] rcu: Replace 1 by true
2020-03-30 1:24 ` [PATCH v2 2/4] rcu: Replace 1 by true Jules Irenge
@ 2020-03-30 21:16 ` Paul E. McKenney
0 siblings, 0 replies; 21+ messages in thread
From: Paul E. McKenney @ 2020-03-30 21:16 UTC (permalink / raw)
To: Jules Irenge
Cc: linux-kernel, julia.lawall, boqun.feng, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes,
open list:READ-COPY UPDATE (RCU)
On Mon, Mar 30, 2020 at 02:24:48AM +0100, Jules Irenge wrote:
> Coccinelle reports a warning at use_softirq declaration
>
> WARNING: Assignment of 0/1 to bool variable
>
> The root cause is
> use_softirq a variable of bool type is initialised with the integer 1
> Replacing 1 with value true solve the issue.
>
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
Queued for review and testing, thank you!
Thanx, Paul
> ---
> kernel/rcu/tree.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index d91c9156fab2..c2ffea31eae8 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -100,7 +100,7 @@ static struct rcu_state rcu_state = {
> static bool dump_tree;
> module_param(dump_tree, bool, 0444);
> /* By default, use RCU_SOFTIRQ instead of rcuc kthreads. */
> -static bool use_softirq = 1;
> +static bool use_softirq = true;
> module_param(use_softirq, bool, 0444);
> /* Control rcu_node-tree auto-balancing at boot time. */
> static bool rcu_fanout_exact;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool
2020-03-30 11:21 ` Peter Zijlstra
@ 2020-04-03 16:17 ` Jules Irenge
0 siblings, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-04-03 16:17 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Jules Irenge, linux-kernel, julia.lawall, boqun.feng,
Ingo Molnar, Will Deacon
On Mon, 30 Mar 2020, Peter Zijlstra wrote:
> On Mon, Mar 30, 2020 at 02:24:50AM +0100, Jules Irenge wrote:
>> Coccinelle reports a warning inside __sched rt_mutex_slowunlock()
>>
>> WARNING: Comparison to bool
>
> I don't mind the patch; but WTH is that a WARNING ?!? Superfluous, but
> definitely not wrong or even dangerous AFAICT.
>
>> To fix this,
>> a comparison (==) of a bool type function result to value true
>> together with the value are removed.
>>
>> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
>> ---
>> kernel/locking/rtmutex.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
>> index 851bbb10819d..7289e7b26be4 100644
>> --- a/kernel/locking/rtmutex.c
>> +++ b/kernel/locking/rtmutex.c
>> @@ -1378,7 +1378,7 @@ static bool __sched rt_mutex_slowunlock(struct rt_mutex *lock,
>> */
>> while (!rt_mutex_has_waiters(lock)) {
>> /* Drops lock->wait_lock ! */
>> - if (unlock_rt_mutex_safe(lock, flags) == true)
>> + if (unlock_rt_mutex_safe(lock, flags))
>> return false;
>> /* Relock the rtmutex and try again */
>> raw_spin_lock_irqsave(&lock->wait_lock, flags);
>> --
>> 2.25.1
>>
>
Thanks for the reply, I will take good note.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/4] Checkpatch tool errors clean up
[not found] <0/4>
2020-03-30 1:24 ` [PATCH v2 0/4] Coccinelle cleanups Jules Irenge
@ 2020-08-01 18:45 ` Jules Irenge
2020-08-01 18:46 ` [PATCH 1/4] acct: Add required space between variable and operator Jules Irenge
` (4 more replies)
1 sibling, 5 replies; 21+ messages in thread
From: Jules Irenge @ 2020-08-01 18:45 UTC (permalink / raw)
To: linux-kernel; +Cc: Jules Irenge
Hi
I am proposing these 4 patches.
I am currently learning the core kernel the hard way.
I will appreciate any feedback negative or positive.
Thanks
Jules Irenge (4):
acct: Add required space between variable and operator
audit: uninitialize global variable audit_sig_sid
audit: uninitialize static variables
context_tracking: uninitialize static variables
kernel/acct.c | 2 +-
kernel/audit.c | 10 +++++-----
kernel/context_tracking.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/4] acct: Add required space between variable and operator
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
@ 2020-08-01 18:46 ` Jules Irenge
2020-08-01 18:46 ` [PATCH 2/4] audit: uninitialize global variable audit_sig_sid Jules Irenge
` (3 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-08-01 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jules Irenge, Michel Lespinasse, Vlastimil Babka, Daniel Jordan,
Andrew Morton, Arnd Bergmann
Checkpatch tool reports an error
"ERROR: spaces required around that == (ctx:VxV)"
To fix this space has been added between the variable,
the operator and the value.
Add the missing required space.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/acct.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/acct.c b/kernel/acct.c
index b0c5b3a9f5af..d7cc5f917e11 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -451,7 +451,7 @@ static void fill_ac(acct_t *ac)
do_div(elapsed, AHZ);
btime = ktime_get_real_seconds() - elapsed;
ac->ac_btime = clamp_t(time64_t, btime, 0, U32_MAX);
-#if ACCT_VERSION==2
+#if ACCT_VERSION == 2
ac->ac_ahz = AHZ;
#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/4] audit: uninitialize global variable audit_sig_sid
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
2020-08-01 18:46 ` [PATCH 1/4] acct: Add required space between variable and operator Jules Irenge
@ 2020-08-01 18:46 ` Jules Irenge
2020-08-01 18:55 ` Joe Perches
2020-08-01 18:46 ` [PATCH 3/4] audit: uninitialize static variables Jules Irenge
` (2 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Jules Irenge @ 2020-08-01 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jules Irenge, Paul Moore, Eric Paris, moderated list:AUDIT SUBSYSTEM
Checkpatch tool reports an error at variable audit_sig_sid declaration
"ERROR: do not initialise globals to 0"
To fix this, the global variable has been uninitialized.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 8c201f414226..7b1a38a211a9 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. */
kuid_t audit_sig_uid = INVALID_UID;
pid_t audit_sig_pid = -1;
-u32 audit_sig_sid = 0;
+u32 audit_sig_sid;
/* Records can be lost in several ways:
0) [suppressed in audit_alloc]
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] audit: uninitialize static variables
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
2020-08-01 18:46 ` [PATCH 1/4] acct: Add required space between variable and operator Jules Irenge
2020-08-01 18:46 ` [PATCH 2/4] audit: uninitialize global variable audit_sig_sid Jules Irenge
@ 2020-08-01 18:46 ` Jules Irenge
2020-08-01 20:21 ` Paul Moore
2020-08-01 18:46 ` [PATCH 4/4] context_tracking: " Jules Irenge
2020-08-01 19:07 ` [PATCH 0/4] Checkpatch tool errors clean up Joe Perches
4 siblings, 1 reply; 21+ messages in thread
From: Jules Irenge @ 2020-08-01 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jules Irenge, Paul Moore, Eric Paris, moderated list:AUDIT SUBSYSTEM
Checkpatch tool reports an error at variable declaration
"ERROR: do not initialise statics to 0"
This is due to the fact that these variables are stored in the buffer
In the .bss section, one can not set an initial value
Here we can trust the compiler to automatically set them to zero.
The variable has since been uninitialized.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/audit.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 7b1a38a211a9..7d79ecb58b01 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -311,8 +311,8 @@ void audit_panic(const char *message)
static inline int audit_rate_check(void)
{
- static unsigned long last_check = 0;
- static int messages = 0;
+ static unsigned long last_check;
+ static int messages;
static DEFINE_SPINLOCK(lock);
unsigned long flags;
unsigned long now;
@@ -348,7 +348,7 @@ static inline int audit_rate_check(void)
*/
void audit_log_lost(const char *message)
{
- static unsigned long last_msg = 0;
+ static unsigned long last_msg;
static DEFINE_SPINLOCK(lock);
unsigned long flags;
unsigned long now;
@@ -713,7 +713,7 @@ static int kauditd_send_queue(struct sock *sk, u32 portid,
{
int rc = 0;
struct sk_buff *skb;
- static unsigned int failed = 0;
+ static unsigned int failed;
/* NOTE: kauditd_thread takes care of all our locking, we just use
* the netlink info passed to us (e.g. sk and portid) */
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/4] context_tracking: uninitialize static variables
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
` (2 preceding siblings ...)
2020-08-01 18:46 ` [PATCH 3/4] audit: uninitialize static variables Jules Irenge
@ 2020-08-01 18:46 ` Jules Irenge
2020-08-03 9:37 ` peterz
2020-08-01 19:07 ` [PATCH 0/4] Checkpatch tool errors clean up Joe Perches
4 siblings, 1 reply; 21+ messages in thread
From: Jules Irenge @ 2020-08-01 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jules Irenge, Peter Zijlstra, Frederic Weisbecker,
Thomas Gleixner, Alexandre Chartre, Masami Hiramatsu
Checkpatch tool reports an error at a staic variable declaration
"ERROR: do not initialise statics to false"
This is due to the fact that this variable is stored in the buffer
In the .bss section, one can not set an initial value
Here we can trust the compiler to automatically set them to false.
The variable has since been uninitialized.
Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
kernel/context_tracking.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 36a98c48aedc..21881c534152 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -190,7 +190,7 @@ NOKPROBE_SYMBOL(context_tracking_user_exit);
void __init context_tracking_cpu_set(int cpu)
{
- static __initdata bool initialized = false;
+ static __initdata bool initialized;
if (!per_cpu(context_tracking.active, cpu)) {
per_cpu(context_tracking.active, cpu) = true;
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] audit: uninitialize global variable audit_sig_sid
2020-08-01 18:46 ` [PATCH 2/4] audit: uninitialize global variable audit_sig_sid Jules Irenge
@ 2020-08-01 18:55 ` Joe Perches
2020-08-01 20:26 ` Paul Moore
2020-08-02 14:32 ` Jules Irenge
0 siblings, 2 replies; 21+ messages in thread
From: Joe Perches @ 2020-08-01 18:55 UTC (permalink / raw)
To: Jules Irenge, linux-kernel
Cc: Paul Moore, Eric Paris, moderated list:AUDIT SUBSYSTEM
On Sat, 2020-08-01 at 19:46 +0100, Jules Irenge wrote:
> Checkpatch tool reports an error at variable audit_sig_sid declaration
[]
> diff --git 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. */
> kuid_t audit_sig_uid = INVALID_UID;
> pid_t audit_sig_pid = -1;
> -u32 audit_sig_sid = 0;
> +u32 audit_sig_sid;
All of these are unused outside of audit.c and might as
well be static and removed from the .h file.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] Checkpatch tool errors clean up
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
` (3 preceding siblings ...)
2020-08-01 18:46 ` [PATCH 4/4] context_tracking: " Jules Irenge
@ 2020-08-01 19:07 ` Joe Perches
2020-08-02 14:54 ` Jules Irenge
4 siblings, 1 reply; 21+ messages in thread
From: Joe Perches @ 2020-08-01 19:07 UTC (permalink / raw)
To: Jules Irenge, linux-kernel
On Sat, 2020-08-01 at 19:45 +0100, Jules Irenge wrote:
> Hi
> I am proposing these 4 patches.
> I am currently learning the core kernel the hard way.
> I will appreciate any feedback negative or positive.
> Thanks
Generally, whitespace only changes outside of drivers/staging
are not encouraged.
> Jules Irenge (4):
> acct: Add required space between variable and operator
> audit: uninitialize global variable audit_sig_sid
> audit: uninitialize static variables
> context_tracking: uninitialize static variables
>
> kernel/acct.c | 2 +-
> kernel/audit.c | 10 +++++-----
> kernel/context_tracking.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/4] audit: uninitialize static variables
2020-08-01 18:46 ` [PATCH 3/4] audit: uninitialize static variables Jules Irenge
@ 2020-08-01 20:21 ` Paul Moore
0 siblings, 0 replies; 21+ messages in thread
From: Paul Moore @ 2020-08-01 20:21 UTC (permalink / raw)
To: Jules Irenge; +Cc: linux-kernel, Eric Paris, moderated list:AUDIT SUBSYSTEM
On Sat, Aug 1, 2020 at 2:46 PM Jules Irenge <jbi.octave@gmail.com> wrote:
>
> Checkpatch tool reports an error at variable declaration
>
> "ERROR: do not initialise statics to 0"
>
> This is due to the fact that these variables are stored in the buffer
> In the .bss section, one can not set an initial value
> Here we can trust the compiler to automatically set them to zero.
> The variable has since been uninitialized.
>
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> kernel/audit.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
In general this is fine, but it will have to wait until after the
merge window closes.
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 7b1a38a211a9..7d79ecb58b01 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -311,8 +311,8 @@ void audit_panic(const char *message)
>
> static inline int audit_rate_check(void)
> {
> - static unsigned long last_check = 0;
> - static int messages = 0;
> + static unsigned long last_check;
> + static int messages;
> static DEFINE_SPINLOCK(lock);
> unsigned long flags;
> unsigned long now;
> @@ -348,7 +348,7 @@ static inline int audit_rate_check(void)
> */
> void audit_log_lost(const char *message)
> {
> - static unsigned long last_msg = 0;
> + static unsigned long last_msg;
> static DEFINE_SPINLOCK(lock);
> unsigned long flags;
> unsigned long now;
> @@ -713,7 +713,7 @@ static int kauditd_send_queue(struct sock *sk, u32 portid,
> {
> int rc = 0;
> struct sk_buff *skb;
> - static unsigned int failed = 0;
> + static unsigned int failed;
>
> /* NOTE: kauditd_thread takes care of all our locking, we just use
> * the netlink info passed to us (e.g. sk and portid) */
> --
> 2.26.2
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] audit: uninitialize global variable audit_sig_sid
2020-08-01 18:55 ` Joe Perches
@ 2020-08-01 20:26 ` Paul Moore
2020-08-02 14:32 ` Jules Irenge
1 sibling, 0 replies; 21+ messages in thread
From: Paul Moore @ 2020-08-01 20:26 UTC (permalink / raw)
To: Joe Perches, Jules Irenge
Cc: linux-kernel, Eric Paris, moderated list:AUDIT SUBSYSTEM
On Sat, Aug 1, 2020 at 2:55 PM Joe Perches <joe@perches.com> wrote:
> On Sat, 2020-08-01 at 19:46 +0100, Jules Irenge wrote:
> > Checkpatch tool reports an error at variable audit_sig_sid declaration
> []
> > diff --git 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. */
> > kuid_t audit_sig_uid = INVALID_UID;
> > pid_t audit_sig_pid = -1;
> > -u32 audit_sig_sid = 0;
> > +u32 audit_sig_sid;
>
> All of these are unused outside of audit.c and might as
> well be static and removed from the .h file.
There's plenty of time before the merge window closes, doing this
would definitely make this patch much more useful than the typical
checkpatch noise.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] audit: uninitialize global variable audit_sig_sid
2020-08-01 18:55 ` Joe Perches
2020-08-01 20:26 ` Paul Moore
@ 2020-08-02 14:32 ` Jules Irenge
1 sibling, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-08-02 14:32 UTC (permalink / raw)
To: Joe Perches
Cc: Jules Irenge, linux-kernel, Paul Moore, Eric Paris,
moderated list:AUDIT SUBSYSTEM
On Sat, 1 Aug 2020, Joe Perches wrote:
> On Sat, 2020-08-01 at 19:46 +0100, Jules Irenge wrote:
> > Checkpatch tool reports an error at variable audit_sig_sid declaration
> []
> > diff --git 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. */
> > kuid_t audit_sig_uid = INVALID_UID;
> > pid_t audit_sig_pid = -1;
> > -u32 audit_sig_sid = 0;
> > +u32 audit_sig_sid;
>
> All of these are unused outside of audit.c and might as
> well be static and removed from the .h file.
>
Thanks for reply, I will resend a second version with the recommendation,
namely make the above static and remove them from the .h file.
Jules
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] Checkpatch tool errors clean up
2020-08-01 19:07 ` [PATCH 0/4] Checkpatch tool errors clean up Joe Perches
@ 2020-08-02 14:54 ` Jules Irenge
0 siblings, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-08-02 14:54 UTC (permalink / raw)
To: Joe Perches; +Cc: Jules Irenge, linux-kernel
On Sat, 1 Aug 2020, Joe Perches wrote:
> On Sat, 2020-08-01 at 19:45 +0100, Jules Irenge wrote:
> > Hi
> > I am proposing these 4 patches.
> > I am currently learning the core kernel the hard way.
> > I will appreciate any feedback negative or positive.
> > Thanks
>
> Generally, whitespace only changes outside of drivers/staging
> are not encouraged.
>
> > Jules Irenge (4):
> > acct: Add required space between variable and operator
> > audit: uninitialize global variable audit_sig_sid
> > audit: uninitialize static variables
> > context_tracking: uninitialize static variables
> >
> > kernel/acct.c | 2 +-
> > kernel/audit.c | 10 +++++-----
> > kernel/context_tracking.c | 2 +-
> > 3 files changed, 7 insertions(+), 7 deletions(-)
> >
>
Thanks, I take good note.
Kind regards,
Jules
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] context_tracking: uninitialize static variables
2020-08-01 18:46 ` [PATCH 4/4] context_tracking: " Jules Irenge
@ 2020-08-03 9:37 ` peterz
2020-08-03 10:27 ` Jules Irenge
0 siblings, 1 reply; 21+ messages in thread
From: peterz @ 2020-08-03 9:37 UTC (permalink / raw)
To: Jules Irenge
Cc: linux-kernel, Frederic Weisbecker, Thomas Gleixner,
Alexandre Chartre, Masami Hiramatsu
On Sat, Aug 01, 2020 at 07:46:03PM +0100, Jules Irenge wrote:
> Checkpatch tool reports an error at a staic variable declaration
>
> "ERROR: do not initialise statics to false"
>
> This is due to the fact that this variable is stored in the buffer
> In the .bss section, one can not set an initial value
> Here we can trust the compiler to automatically set them to false.
> The variable has since been uninitialized.
>
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
> kernel/context_tracking.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
> index 36a98c48aedc..21881c534152 100644
> --- a/kernel/context_tracking.c
> +++ b/kernel/context_tracking.c
> @@ -190,7 +190,7 @@ NOKPROBE_SYMBOL(context_tracking_user_exit);
>
> void __init context_tracking_cpu_set(int cpu)
> {
> - static __initdata bool initialized = false;
> + static __initdata bool initialized;
So personally I prefer having the '= false' there. It used to be that
compilers were stupid and would put any initialized static variable in
.data, even if it was initialized with 0. But AFAIK compilers are no
longer that stupid.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/4] context_tracking: uninitialize static variables
2020-08-03 9:37 ` peterz
@ 2020-08-03 10:27 ` Jules Irenge
0 siblings, 0 replies; 21+ messages in thread
From: Jules Irenge @ 2020-08-03 10:27 UTC (permalink / raw)
To: peterz
Cc: Jules Irenge, linux-kernel, Frederic Weisbecker, Thomas Gleixner,
Alexandre Chartre, Masami Hiramatsu
On Mon, 3 Aug 2020, peterz@infradead.org wrote:
> So personally I prefer having the '= false' there. It used to be that
> compilers were stupid and would put any initialized static variable in
> .data, even if it was initialized with 0. But AFAIK compilers are no
> longer that stupid.
>
Thanks for the reply, I completely understand the precaution measure.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-08-03 10:28 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <0/4>
2020-03-30 1:24 ` [PATCH v2 0/4] Coccinelle cleanups Jules Irenge
2020-03-30 1:24 ` [PATCH v2 1/4] cpu: Remove Comparison to bool Jules Irenge
2020-03-30 1:24 ` [PATCH v2 2/4] rcu: Replace 1 by true Jules Irenge
2020-03-30 21:16 ` Paul E. McKenney
2020-03-30 1:24 ` [PATCH v2 3/4] genirq: " Jules Irenge
2020-03-30 1:24 ` [PATCH v2 4/4] locking/rtmutex: Remove Comparison to bool Jules Irenge
2020-03-30 11:21 ` Peter Zijlstra
2020-04-03 16:17 ` Jules Irenge
2020-08-01 18:45 ` [PATCH 0/4] Checkpatch tool errors clean up Jules Irenge
2020-08-01 18:46 ` [PATCH 1/4] acct: Add required space between variable and operator Jules Irenge
2020-08-01 18:46 ` [PATCH 2/4] audit: uninitialize global variable audit_sig_sid Jules Irenge
2020-08-01 18:55 ` Joe Perches
2020-08-01 20:26 ` Paul Moore
2020-08-02 14:32 ` Jules Irenge
2020-08-01 18:46 ` [PATCH 3/4] audit: uninitialize static variables Jules Irenge
2020-08-01 20:21 ` Paul Moore
2020-08-01 18:46 ` [PATCH 4/4] context_tracking: " Jules Irenge
2020-08-03 9:37 ` peterz
2020-08-03 10:27 ` Jules Irenge
2020-08-01 19:07 ` [PATCH 0/4] Checkpatch tool errors clean up Joe Perches
2020-08-02 14:54 ` Jules Irenge
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).