* [PATCH] handle more graciously labels with no statement
@ 2020-10-26 4:53 Luc Van Oostenryck
2020-10-26 17:41 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2020-10-26 4:53 UTC (permalink / raw)
To: linux-sparse; +Cc: Luc Van Oostenryck
In C a label must precede a statement. A null statement is OK
but a closing braces is not.
So, catch this situation, emit a warning and continue as if a
null statement was there.
This occurs currently on v5.10-rc1 because of some ifdefery.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
parse.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/parse.c b/parse.c
index 31ecef0f554d..b6090d38cc61 100644
--- a/parse.c
+++ b/parse.c
@@ -2468,6 +2468,11 @@ static struct token *statement(struct token *token, struct statement **tree)
warn_label_usage(stmt->pos, s->label_pos, s->ident);
}
s->stmt = stmt;
+ if (match_op(token, '}')) {
+ warning(token->pos, "statement expected after label");
+ stmt->label_statement = alloc_statement(token->pos, STMT_NONE);
+ return token;
+ }
return statement(token, &stmt->label_statement);
}
}
--
2.29.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] handle more graciously labels with no statement
2020-10-26 4:53 [PATCH] handle more graciously labels with no statement Luc Van Oostenryck
@ 2020-10-26 17:41 ` Linus Torvalds
2020-10-26 20:48 ` Luc Van Oostenryck
0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2020-10-26 17:41 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Sparse Mailing-list
On Sun, Oct 25, 2020 at 9:53 PM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> This occurs currently on v5.10-rc1 because of some ifdefery.
Well, sparse being more robust is good, so the patch looks sane to me..
But can you point to the actual 5.10-rc1 problem? I haven't seen the
report, and afaik gcc will complain about this too ("label at end of
compound statement") so I'm surprised sparse hits it..
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] handle more graciously labels with no statement
2020-10-26 17:41 ` Linus Torvalds
@ 2020-10-26 20:48 ` Luc Van Oostenryck
2020-10-26 22:56 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2020-10-26 20:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Sparse Mailing-list
On Mon, Oct 26, 2020 at 10:41:36AM -0700, Linus Torvalds wrote:
> On Sun, Oct 25, 2020 at 9:53 PM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> >
> > This occurs currently on v5.10-rc1 because of some ifdefery.
>
> Well, sparse being more robust is good, so the patch looks sane to me..
>
> But can you point to the actual 5.10-rc1 problem? I haven't seen the
> report, and afaik gcc will complain about this too ("label at end of
> compound statement") so I'm surprised sparse hits it..
Well, I was surprised it wasn't caught and was ready to send a patch
but it can only happen when using sparse, see below, quite ironical:
drivers/scsi/qla2xxx/qla_tmpl.c:1052.
1050 }
1051
1052 bailout:
1053 #ifndef __CHECKER__
1054 if (!hardware_locked)
1055 spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
1056 #endif
1057 }
-- Luc
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] handle more graciously labels with no statement
2020-10-26 20:48 ` Luc Van Oostenryck
@ 2020-10-26 22:56 ` Linus Torvalds
2020-10-26 23:22 ` Bart Van Assche
0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2020-10-26 22:56 UTC (permalink / raw)
To: Luc Van Oostenryck, Bart Van Assche, Arun Easi; +Cc: Sparse Mailing-list
On Mon, Oct 26, 2020 at 1:48 PM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> On Mon, Oct 26, 2020 at 10:41:36AM -0700, Linus Torvalds wrote:
> > But can you point to the actual 5.10-rc1 problem? I haven't seen the
> > report, and afaik gcc will complain about this too ("label at end of
> > compound statement") so I'm surprised sparse hits it..
>
> Well, I was surprised it wasn't caught and was ready to send a patch
> but it can only happen when using sparse, see below, quite ironic:
> drivers/scsi/qla2xxx/qla_tmpl.c:1052.
> 1050 }
> 1051
> 1052 bailout:
> 1053 #ifndef __CHECKER__
> 1054 if (!hardware_locked)
> 1055 spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
> 1056 #endif
> 1057 }
Yeah, that "don't sparse this" pattern has been problematic before.
I removed it.
Bart, Arun - see commit 4525c8781ec0 ("scsi: qla2xxx: remove incorrect
sparse #ifdef").
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] handle more graciously labels with no statement
2020-10-26 22:56 ` Linus Torvalds
@ 2020-10-26 23:22 ` Bart Van Assche
2020-10-26 23:39 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2020-10-26 23:22 UTC (permalink / raw)
To: Linus Torvalds, Luc Van Oostenryck, Arun Easi; +Cc: Sparse Mailing-list
On 10/26/20 3:56 PM, Linus Torvalds wrote:
> On Mon, Oct 26, 2020 at 1:48 PM Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>>
>> On Mon, Oct 26, 2020 at 10:41:36AM -0700, Linus Torvalds wrote:
>>> But can you point to the actual 5.10-rc1 problem? I haven't seen the
>>> report, and afaik gcc will complain about this too ("label at end of
>>> compound statement") so I'm surprised sparse hits it..
>>
>> Well, I was surprised it wasn't caught and was ready to send a patch
>> but it can only happen when using sparse, see below, quite ironic:
>> drivers/scsi/qla2xxx/qla_tmpl.c:1052.
>> 1050 }
>> 1051
>> 1052 bailout:
>> 1053 #ifndef __CHECKER__
>> 1054 if (!hardware_locked)
>> 1055 spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
>> 1056 #endif
>> 1057 }
>
> Yeah, that "don't sparse this" pattern has been problematic before.
>
> I removed it.
>
> Bart, Arun - see commit 4525c8781ec0 ("scsi: qla2xxx: remove incorrect
> sparse #ifdef").
Hi Linus,
Thank you for having Cc-ed me. I agree that it's better to make the
kernel code compliant with the C standard than to make sparse accept
non-standard code.
The #ifndef __CHECKER__ / #endif that I added in 2015 in the above code
(commit 8d16366b5f23) was added because at that time I didn't see a
better solution.
BTW, personally I'm neither enthusiast about #ifndef __CHECKER__ /
#endif nor about if (expression) <locking statement>.
Bart.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] handle more graciously labels with no statement
2020-10-26 23:22 ` Bart Van Assche
@ 2020-10-26 23:39 ` Linus Torvalds
0 siblings, 0 replies; 6+ messages in thread
From: Linus Torvalds @ 2020-10-26 23:39 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Luc Van Oostenryck, Arun Easi, Sparse Mailing-list
On Mon, Oct 26, 2020 at 4:22 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> The #ifndef __CHECKER__ / #endif that I added in 2015 in the above code
> (commit 8d16366b5f23) was added because at that time I didn't see a
> better solution.
I think there are only a couple of callers, and all of them just have
a constant "device_locked" argument.
It should be easy to make the (I think single) use that _didn't_ lock
the device just do the locking, and then all of them have
"device_locked = 1", and then that argument can be removed and the
whole conditional be replaced by a lockdep assert.
Exactly like y9ou did in 8ae178760b23 ("scsi: qla2xxx: Simplify the
functions for dumping firmware") in other words.
But I didn't really look _that_ much into it, this is just from a
simple "grep" thing and maybe I missed something. I just did the
minimal "don't do invalid C" thing.
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-26 23:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-26 4:53 [PATCH] handle more graciously labels with no statement Luc Van Oostenryck
2020-10-26 17:41 ` Linus Torvalds
2020-10-26 20:48 ` Luc Van Oostenryck
2020-10-26 22:56 ` Linus Torvalds
2020-10-26 23:22 ` Bart Van Assche
2020-10-26 23:39 ` Linus Torvalds
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).