misc: fix returning void-valued expression warnings
Subsystems (2)
PATCH 9/10 misc: fix returning void-valued expression warnings
kernel/sched_fair.c:845:3: warning: returning void-valued expression kernel/sched.c:4343:3: warning: returning void-valued expression security/security.c:897:2: warning: returning void-valued expression security/security.c:1014:2: warning: returning void-valued expression security/security.c:1019:2: warning: returning void-valued expression sound/sound_core.c:410:2: warning: returning void-valued expression sound/sound_core.c:427:2: warning: returning void-valued expression
---
Peter, I know you don't like this very much, but it would help reduce the trivial noise somewhat in a sparse build.
kernel/sched.c | 6 ++++-- kernel/sched_fair.c | 6 ++++-- security/security.c | 6 +++--- sound/sound_core.c | 4 ++-- 4 files changed, 13 insertions(+), 9 deletions(-)
kernel/sched.c +4 -2
@@ -4339,8 +4339,10 @@ void account_system_time(struct task_struct *p, int hardirq_offset,struct rq *rq =this_rq ();cputime64_t tmp; -if ((p->flags &PF_VCPU ) && (irq_count () - hardirq_offset ==0 )) -return account_guest_time (p, cputime); +if ((p->flags &PF_VCPU ) && (irq_count () - hardirq_offset ==0 )) { +account_guest_time (p, cputime); +return ; + } p->stime =cputime_add (p->stime, cputime);
kernel/sched_fair.c +4 -2
@@ -841,8 +841,10 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)* queued ticks are scheduled to match the slice, so don't bother * validating it and just reschedule. */ -if (queued) -return resched_task (rq_of (cfs_rq)->curr); +if (queued) { +resched_task (rq_of (cfs_rq)->curr); +return ; + }/* * don't let the period tick interfere with the hrtick preemption */
security/security.c +3 -3
@@ -894,7 +894,7 @@ EXPORT_SYMBOL(security_secctx_to_secid);void security_release_secctx (char *secdata,u32 seclen) { -return security_ops->release_secctx (secdata, seclen); + security_ops->release_secctx (secdata, seclen); }EXPORT_SYMBOL (security_release_secctx);
@@ -1011,12 +1011,12 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority)void security_sk_free (struct sock *sk) { -return security_ops->sk_free_security (sk); + security_ops->sk_free_security (sk); }void security_sk_clone (const struct sock *sk,struct sock *newsk) { -return security_ops->sk_clone_security (sk, newsk); + security_ops->sk_clone_security (sk, newsk); }void security_sk_classify_flow (struct sock *sk,struct flowi *fl)
sound/sound_core.c +2 -2
@@ -407,7 +407,7 @@ EXPORT_SYMBOL(unregister_sound_mixer);void unregister_sound_midi (int unit) { -return sound_remove_unit (&chains[2 ], unit); +sound_remove_unit (&chains[2 ], unit); }EXPORT_SYMBOL (unregister_sound_midi);
@@ -424,7 +424,7 @@ EXPORT_SYMBOL(unregister_sound_midi);void unregister_sound_dsp (int unit) { -return sound_remove_unit (&chains[3 ], unit); +sound_remove_unit (&chains[3 ], unit); }
Signature · 1 line
1.5.5.1.305.g7c84
Quoted 24 lines · Harvey Harrison
kernel/sched_fair.c:845:3: warning: returning void-valued expressionkernel/sched.c:4343:3: warning: returning void-valued expressionsecurity/security.c:897:2: warning: returning void-valued expressionsecurity/security.c:1014:2: warning: returning void-valued expressionsecurity/security.c:1019:2: warning: returning void-valued expressionsound/sound_core.c:410:2: warning: returning void-valued expressionsound/sound_core.c:427:2: warning: returning void-valued expressionSigned-off-by Harvey Harrison <[email protected]>---
Peter, I know you don't like this very much, but it would help reduce the trivial noise somewhat in a sparse build.kernel/sched.c | 6 ++++-- kernel/sched_fair.c | 6 ++++-- security/security.c | 6 +++--- sound/sound_core.c | 4 ++-- 4 files changed, 13 insertions(+), 9 deletions(-)kernel/sched.c +0
-0
kernel/sched.c void account_system_time(struct task_struct *p, int hardirq_offset, · line 4345
Quoted 9 lines
kernel/sched.c +4
-2@@ -4339,8 +4339,10 @@ void account_system_time(struct task_struct *p, int hardirq_offset,struct rq *rq =this_rq ();cputime64_t tmp; -if ((p->flags &PF_VCPU ) && (irq_count () - hardirq_offset ==0 )) -return account_guest_time (p, cputime); +if ((p->flags &PF_VCPU ) && (irq_count () - hardirq_offset ==0 )) { +account_guest_time (p, cputime); +return ; + }
I don't know who invented sparse, but I like this form of return. 1 - It saves me the curly brackets and extra return line. But mainly 2 - It is a programing statement that says: "Me here I'm an equivalent
to that other call". So if in the future that inner function starts to return, say, an error value, with the first style the compiler will error. But with the second style the new error return will be silently ignored. So these are not equivalent replacements. The former is a much stronger bond between the caller and the callie.
Quoted 62 lines
kernel/sched.c +0
-0p->stime =cputime_add (p->stime, cputime);kernel/sched_fair.c +4
-2@@ -841,8 +841,10 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)* queued ticks are scheduled to match the slice, so don't bother * validating it and just reschedule. */ -if (queued) -return resched_task (rq_of (cfs_rq)->curr); +if (queued) { +resched_task (rq_of (cfs_rq)->curr); +return ; + }/* * don't let the period tick interfere with the hrtick preemption */ security/security.c +3
-3@@ -894,7 +894,7 @@ EXPORT_SYMBOL(security_secctx_to_secid);void security_release_secctx (char *secdata,u32 seclen) { -return security_ops->release_secctx (secdata, seclen); + security_ops->release_secctx (secdata, seclen); }EXPORT_SYMBOL (security_release_secctx);@@ -1011,12 +1011,12 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority)void security_sk_free (struct sock *sk) { -return security_ops->sk_free_security (sk); + security_ops->sk_free_security (sk); }void security_sk_clone (const struct sock *sk,struct sock *newsk) { -return security_ops->sk_clone_security (sk, newsk); + security_ops->sk_clone_security (sk, newsk); }void security_sk_classify_flow (struct sock *sk,struct flowi *fl)sound/sound_core.c +1
-1@@ -407,7 +407,7 @@ EXPORT_SYMBOL(unregister_sound_mixer);void unregister_sound_midi (int unit) { -return sound_remove_unit (&chains[2 ], unit); +sound_remove_unit (&chains[2 ], unit); }EXPORT_SYMBOL (unregister_sound_midi);
sound/sound_core.c EXPORT_SYMBOL(unregister_sound_midi); · line 430
Quoted 8 lines
sound/sound_core.c +1
-1@@ -424,7 +424,7 @@ EXPORT_SYMBOL(unregister_sound_midi);void unregister_sound_dsp (int unit) { -return sound_remove_unit (&chains[3 ], unit); +sound_remove_unit (&chains[3 ], unit); }
Even without the saving in the if case, I would say fix sparse, it is very very wrong. a return (void)do(); is not equal to do(); return;
Code is not only high level assembly, it is also a low level English. In English the first form is telling what the future assembly should be.
big NACK from me (== $0.02)
Boaz
Quoted 8 lines · Boaz Harrosh
I don't know who invented sparse, but I like this form of return. 1 - It saves me the curly brackets and extra return line. But mainly 2 - It is a programing statement that says: "Me here I'm an equivalent
to that other call". So if in the future that inner function starts to return, say, an error value, with the first style the compiler will error. But with the second style the new error return will be silently ignored. So these are not equivalent replacements. The former is a much stronger bond between the caller and the callie.
3. 6.8.6.4(1): A return statement with an expression shall not appear in a function whose return type is void.
Write in C, please.
Quoted 11 lines · Al Viro
Quoted 8 lines · Boaz Harrosh
I don't know who invented sparse, but I like this form of return. 1 - It saves me the curly brackets and extra return line. But mainly 2 - It is a programing statement that says: "Me here I'm an equivalent
to that other call". So if in the future that inner function starts to return, say, an error value, with the first style the compiler will error. But with the second style the new error return will be silently ignored. So these are not equivalent replacements. The former is a much stronger bond between the caller and the callie.
Quoted 4 lines
3. 6.8.6.4(1): A return statement with an expression shall not appear in a function whose return type is void.
Write in C, please.
We use GNU99 all over the place, or are you going to clear up all the statement expressions and such other fancy gnu extensions to the language as well?
I'm really not seeing why this would be wrong, other than the standard saying it is, ie. I think the standard got it wrong here.
Harvey can just use -Wno-return-void, or someone can modify sparse to have that default disabled for STANDARD_GNU[89]9.
Quoted 5 lines · Peter Zijlstra
Quoted 1 line
Write in C, please.
We use GNU99 all over the place, or are you going to clear up all the statement expressions and such other fancy gnu extensions to the language as well?
Worthless ones - sure. Statement expressions are not particulary nice one, but we can't avoid that shit (note that they are not even well-defined - you can easily make gcc fall over and die with those).
From: Peter Zijlstra <[email protected]>
Date: Thu, 01 May 2008 14:17:06 +0200
Quoted 21 lines · Al Viro
Quoted 11 lines · Boaz Harrosh
Quoted 9 lines
I don't know who invented sparse, but I like this form of return. 1 - It saves me the curly brackets and extra return line. But mainly 2 - It is a programing statement that says: "Me here I'm an equivalent
to that other call". So if in the future that inner function starts to return, say, an error value, with the first style the compiler will error. But with the second style the new error return will be silently ignored. So these are not equivalent replacements. The former is a much stronger bond between the caller and the callie.Quoted 4 lines
3. 6.8.6.4(1): A return statement with an expression shall not appear in a function whose return type is void.
Write in C, please.
We use GNU99 all over the place, or are you going to clear up all the statement expressions and such other fancy gnu extensions to the language as well?
Quoted 5 lines
I'm really not seeing why this would be wrong, other than the standard saying it is, ie. I think the standard got it wrong here.
Harvey can just use -Wno-return-void, or someone can modify sparse to have that default disabled for STANDARD_GNU[89]9.
Even Linus thinks this construct is fine and said he had a patch to make sparse allow it. This discussion is pointless especially since GCC has allowed this since basically day one, and it's even self consistent.
Quoted 11 lines · Al Viro
Quoted 8 lines · Boaz Harrosh
I don't know who invented sparse, but I like this form of return. 1 - It saves me the curly brackets and extra return line. But mainly 2 - It is a programing statement that says: "Me here I'm an equivalent
to that other call". So if in the future that inner function starts to return, say, an error value, with the first style the compiler will error. But with the second style the new error return will be silently ignored. So these are not equivalent replacements. The former is a much stronger bond between the caller and the callie.
Quoted 3 lines
3. 6.8.6.4(1): A return statement with an expression shall not appear in a function whose return type is void.
Please forgive my ignorance, where is this quote from?
Quoted 1 line
Write in C, please.
I have used this style for ages. I thought it is C, and the compiler never complained. You must agree that the two statements are not equivalent. Is it a bad style? I saw it's merits, perhaps it's just me.
Boaz
Quoted 5 lines · Boaz Harrosh
Quoted 3 lines
3. 6.8.6.4(1): A return statement with an expression shall not appear in a function whose return type is void.
Please forgive my ignorance, where is this quote from?
C99. I don't have C90 in front of me, so I can't give you exact quote from there, but it's been explicitly banned in C90 as well.