strncmp(str, const, len) is error-prone.
We had better use newly introduced
str_has_prefix() instead of it.
Signed-off-by: Chuhong Yuan <[email protected]>
---
kernel/irq/debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index c1eccd4f6520..85d1e9aeb8ea 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -188,7 +188,7 @@ static ssize_t irq_debug_write(struct file *file, const char __user *user_buf,
if (copy_from_user(buf, user_buf, size))
return -EFAULT;
- if (!strncmp(buf, "trigger", size)) {
+ if (str_has_prefix(buf, "trigger")) {
unsigned long flags;
int err;
--
2.20.1
On Mon, 29 Jul 2019, Chuhong Yuan wrote:
> strncmp(str, const, len) is error-prone.
> We had better use newly introduced
> str_has_prefix() instead of it.
Can you please provide a proper explanation why the below strncmp() is
error prone?
Just running a script and copying some boiler plate changelog saying
'strncmp() is error prone' does not cut it.
> - if (!strncmp(buf, "trigger", size)) {
> + if (str_has_prefix(buf, "trigger")) {
Especially when the resulting code is not equivalent.
Thanks,
tglx
On 30/07/2019 11:58, Chuhong Yuan wrote:
> Thomas Gleixner <[email protected]> 于2019年7月30日周二 下午5:17写道：
>>
>> On Mon, 29 Jul 2019, Chuhong Yuan wrote:
>>
>>> strncmp(str, const, len) is error-prone.
>>> We had better use newly introduced
>>> str_has_prefix() instead of it.
>>
>> Can you please provide a proper explanation why the below strncmp() is
>> error prone?
>>
>
> If the size is less than 7, for example, 2, then even if buf is "tr", the
> result will still be true. This is an error.
> strncmp(str, const, len) is error-prone mainly because the len is easy
> to be wrong.
>
>> Just running a script and copying some boiler plate changelog saying
>> 'strncmp() is error prone' does not cut it.
>>
>>> - if (!strncmp(buf, "trigger", size)) {
>>> + if (str_has_prefix(buf, "trigger")) {
>>
>> Especially when the resulting code is not equivalent.
>>
>
> I think here the semantic is the comparison should only return true
> when buf is "trigger".
Not quite. It will satisfy the condition for 't', 'tr', 'trig',
'trigger', and of course 'triggerthissillyinterruptwhichImdebugging'.
I agree that the semantic is a bit bizarre and maybe not quite expected,
but still... You seem to be changing the semantic without any
justification other than "this is safer".
Thanks,
M.
--
Jazz is not dead, it just smells funny...
Marc Zyngier <[email protected]> 于2019年7月30日周二 下午7:13写道：
>
> On 30/07/2019 11:58, Chuhong Yuan wrote:
> > Thomas Gleixner <[email protected]> 于2019年7月30日周二 下午5:17写道：
> >>
> >> On Mon, 29 Jul 2019, Chuhong Yuan wrote:
> >>
> >>> strncmp(str, const, len) is error-prone.
> >>> We had better use newly introduced
> >>> str_has_prefix() instead of it.
> >>
> >> Can you please provide a proper explanation why the below strncmp() is
> >> error prone?
> >>
> >
> > If the size is less than 7, for example, 2, then even if buf is "tr", the
> > result will still be true. This is an error.
> > strncmp(str, const, len) is error-prone mainly because the len is easy
> > to be wrong.
> >
> >> Just running a script and copying some boiler plate changelog saying
> >> 'strncmp() is error prone' does not cut it.
> >>
> >>> - if (!strncmp(buf, "trigger", size)) {
> >>> + if (str_has_prefix(buf, "trigger")) {
> >>
> >> Especially when the resulting code is not equivalent.
> >>
> >
> > I think here the semantic is the comparison should only return true
> > when buf is "trigger".
>
> Not quite. It will satisfy the condition for 't', 'tr', 'trig',
> 'trigger', and of course 'triggerthissillyinterruptwhichImdebugging'.
>
> I agree that the semantic is a bit bizarre and maybe not quite expected,
> but still... You seem to be changing the semantic without any
> justification other than "this is safer".
>
I am sorry about that... It is my fault.
I will improve my script and avoid such mistakes.
Thanks for your correction.
Regards,
Chuhong
> Thanks,
>
> M.
> --
> Jazz is not dead, it just smells funny...
Thomas Gleixner <[email protected]> 于2019年7月30日周二 下午5:17写道：
>
> On Mon, 29 Jul 2019, Chuhong Yuan wrote:
>
> > strncmp(str, const, len) is error-prone.
> > We had better use newly introduced
> > str_has_prefix() instead of it.
>
> Can you please provide a proper explanation why the below strncmp() is
> error prone?
>
If the size is less than 7, for example, 2, then even if buf is "tr", the
result will still be true. This is an error.
strncmp(str, const, len) is error-prone mainly because the len is easy
to be wrong.
> Just running a script and copying some boiler plate changelog saying
> 'strncmp() is error prone' does not cut it.
>
> > - if (!strncmp(buf, "trigger", size)) {
> > + if (str_has_prefix(buf, "trigger")) {
>
> Especially when the resulting code is not equivalent.
>
I think here the semantic is the comparison should only return true
when buf is "trigger".
The buf's size is 8 and the string's size is at most 7.
Since str_has_prefix()'s implementation is strncmp(str, prefix, strlen(prefix)),
here strlen(prefix) = 7, I think it satisfies the requirement.
Regards,
Chuhong
> Thanks,
>
> tglx