* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-04-25 15:38 ` lizhe
0 siblings, 0 replies; 10+ messages in thread
From: lizhe @ 2022-04-25 15:38 UTC (permalink / raw)
To: bhe, vgoyal, dyoung, prudo, sensor1010; +Cc: kexec, linux-kernel
When ck_cmdline is NULL, the only caller of get_last_crashkernel()
has already done non-NULL check(see __parse_crashkernel()),
so it doesn't make any sense to make a check here
Signed-off-by: lizhe <sensor1010@163.com>
---
kernel/crash_core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 256cf6db573c..c232f01a2c54 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
p = strstr(p+1, name);
}
- if (!ck_cmdline)
- return NULL;
-
return ck_cmdline;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-04-25 15:38 ` lizhe
0 siblings, 0 replies; 10+ messages in thread
From: lizhe @ 2022-04-25 15:38 UTC (permalink / raw)
To: kexec
When ck_cmdline is NULL, the only caller of get_last_crashkernel()
has already done non-NULL check(see __parse_crashkernel()),
so it doesn't make any sense to make a check here
Signed-off-by: lizhe <sensor1010@163.com>
---
kernel/crash_core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 256cf6db573c..c232f01a2c54 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
p = strstr(p+1, name);
}
- if (!ck_cmdline)
- return NULL;
-
return ck_cmdline;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
2022-04-25 15:38 ` lizhe
@ 2022-04-26 8:39 ` Philipp Rudo
-1 siblings, 0 replies; 10+ messages in thread
From: Philipp Rudo @ 2022-04-26 8:39 UTC (permalink / raw)
To: lizhe; +Cc: bhe, vgoyal, dyoung, kexec, linux-kernel
Hi lizhe,
On Mon, 25 Apr 2022 08:38:57 -0700
lizhe <sensor1010@163.com> wrote:
> When ck_cmdline is NULL, the only caller of get_last_crashkernel()
> has already done non-NULL check(see __parse_crashkernel()),
> so it doesn't make any sense to make a check here
sorry, but I still don't like the description. What I don't understand
in particular is why you are mentioning the caller (__parse_crashkernel)
here. ck_cmdline is a local variable to get_last_crashkernel. So the
caller cannot perform any check on the variable but only the return
value of the function. So the patch description should describe why we
can remove the additional return NULL without changing the behavior of
the function.
Thanks
Philipp
> Signed-off-by: lizhe <sensor1010@163.com>
> ---
> kernel/crash_core.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 256cf6db573c..c232f01a2c54 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
> p = strstr(p+1, name);
> }
>
> - if (!ck_cmdline)
> - return NULL;
> -
> return ck_cmdline;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-04-26 8:39 ` Philipp Rudo
0 siblings, 0 replies; 10+ messages in thread
From: Philipp Rudo @ 2022-04-26 8:39 UTC (permalink / raw)
To: kexec
Hi lizhe,
On Mon, 25 Apr 2022 08:38:57 -0700
lizhe <sensor1010@163.com> wrote:
> When ck_cmdline is NULL, the only caller of get_last_crashkernel()
> has already done non-NULL check(see __parse_crashkernel()),
> so it doesn't make any sense to make a check here
sorry, but I still don't like the description. What I don't understand
in particular is why you are mentioning the caller (__parse_crashkernel)
here. ck_cmdline is a local variable to get_last_crashkernel. So the
caller cannot perform any check on the variable but only the return
value of the function. So the patch description should describe why we
can remove the additional return NULL without changing the behavior of
the function.
Thanks
Philipp
> Signed-off-by: lizhe <sensor1010@163.com>
> ---
> kernel/crash_core.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 256cf6db573c..c232f01a2c54 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
> p = strstr(p+1, name);
> }
>
> - if (!ck_cmdline)
> - return NULL;
> -
> return ck_cmdline;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <6a0fa9cc.19d4.1808440ca50.Coremail.sensor1010@163.com>]
* Re: [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
[not found] ` <6a0fa9cc.19d4.1808440ca50.Coremail.sensor1010@163.com>
@ 2022-05-03 16:43 ` Philipp Rudo
0 siblings, 0 replies; 10+ messages in thread
From: Philipp Rudo @ 2022-05-03 16:43 UTC (permalink / raw)
To: lizhe; +Cc: bhe, vgoyal, dyoung, kexec, linux-kernel
Hi lizhe,
On Mon, 2 May 2022 18:11:20 +0800 (CST)
lizhe <sensor1010@163.com> wrote:
> HI Philipp Rudo.
>
>
> When ck_cmdline is NULL. The last three lines of this function are equivalent to :
> if ( ! NULL)
> return NULL;
> return NULL;
> This is obviously a redundant check.
>
>
> I will use the above description to describe the patch,
the explanation looks good to me.
Thanks!
Philipp
>
> thanks.
> lizhe
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2022-04-26 16:39:52, "Philipp Rudo" <prudo@redhat.com> wrote:
> >Hi lizhe,
> >
> >On Mon, 25 Apr 2022 08:38:57 -0700
> >lizhe <sensor1010@163.com> wrote:
> >
> >> When ck_cmdline is NULL, the only caller of get_last_crashkernel()
> >> has already done non-NULL check(see __parse_crashkernel()),
> >> so it doesn't make any sense to make a check here
> >
> >sorry, but I still don't like the description. What I don't understand
> >in particular is why you are mentioning the caller (__parse_crashkernel)
> >here. ck_cmdline is a local variable to get_last_crashkernel. So the
> >caller cannot perform any check on the variable but only the return
> >value of the function. So the patch description should describe why we
> >can remove the additional return NULL without changing the behavior of
> >the function.
> >
> >Thanks
> >Philipp
> >
> >> Signed-off-by: lizhe <sensor1010@163.com>
> >> ---
> >> kernel/crash_core.c | 3 ---
> >> 1 file changed, 3 deletions(-)
> >>
> >> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> >> index 256cf6db573c..c232f01a2c54 100644
> >> --- a/kernel/crash_core.c
> >> +++ b/kernel/crash_core.c
> >> @@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
> >> p = strstr(p+1, name);
> >> }
> >>
> >> - if (!ck_cmdline)
> >> - return NULL;
> >> -
> >> return ck_cmdline;
> >> }
> >>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-05-03 16:43 ` Philipp Rudo
0 siblings, 0 replies; 10+ messages in thread
From: Philipp Rudo @ 2022-05-03 16:43 UTC (permalink / raw)
To: kexec
Hi lizhe,
On Mon, 2 May 2022 18:11:20 +0800 (CST)
lizhe <sensor1010@163.com> wrote:
> HI Philipp Rudo.
>
>
> When ck_cmdline is NULL. The last three lines of this function are equivalent to :
> if ( ! NULL)
> return NULL;
> return NULL;
> This is obviously a redundant check.
>
>
> I will use the above description to describe the patch,
the explanation looks good to me.
Thanks!
Philipp
>
> thanks.
> lizhe
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2022-04-26 16:39:52, "Philipp Rudo" <prudo@redhat.com> wrote:
> >Hi lizhe,
> >
> >On Mon, 25 Apr 2022 08:38:57 -0700
> >lizhe <sensor1010@163.com> wrote:
> >
> >> When ck_cmdline is NULL, the only caller of get_last_crashkernel()
> >> has already done non-NULL check(see __parse_crashkernel()),
> >> so it doesn't make any sense to make a check here
> >
> >sorry, but I still don't like the description. What I don't understand
> >in particular is why you are mentioning the caller (__parse_crashkernel)
> >here. ck_cmdline is a local variable to get_last_crashkernel. So the
> >caller cannot perform any check on the variable but only the return
> >value of the function. So the patch description should describe why we
> >can remove the additional return NULL without changing the behavior of
> >the function.
> >
> >Thanks
> >Philipp
> >
> >> Signed-off-by: lizhe <sensor1010@163.com>
> >> ---
> >> kernel/crash_core.c | 3 ---
> >> 1 file changed, 3 deletions(-)
> >>
> >> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> >> index 256cf6db573c..c232f01a2c54 100644
> >> --- a/kernel/crash_core.c
> >> +++ b/kernel/crash_core.c
> >> @@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
> >> p = strstr(p+1, name);
> >> }
> >>
> >> - if (!ck_cmdline)
> >> - return NULL;
> >> -
> >> return ck_cmdline;
> >> }
> >>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-05-04 12:36 ` lizhe
0 siblings, 0 replies; 10+ messages in thread
From: lizhe @ 2022-05-04 12:36 UTC (permalink / raw)
To: bhe, vgoyal, dyoung, prudo; +Cc: kexec, linux-kernel, lizhe
When ck_cmdline is NULL. The last three lines of
this function(get_last_crashkernel()) are equivalent to :
if (!NULL)
return NULL;
return NULL;
This is obviously a redundant check
Signed-off-by: lizhe <sensor1010@163.com>
---
kernel/crash_core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 256cf6db573c..c232f01a2c54 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
p = strstr(p+1, name);
}
- if (!ck_cmdline)
- return NULL;
-
return ck_cmdline;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-05-04 12:36 ` lizhe
0 siblings, 0 replies; 10+ messages in thread
From: lizhe @ 2022-05-04 12:36 UTC (permalink / raw)
To: kexec
When ck_cmdline is NULL. The last three lines of
this function(get_last_crashkernel()) are equivalent to :
if (!NULL)
return NULL;
return NULL;
This is obviously a redundant check
Signed-off-by: lizhe <sensor1010@163.com>
---
kernel/crash_core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 256cf6db573c..c232f01a2c54 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
p = strstr(p+1, name);
}
- if (!ck_cmdline)
- return NULL;
-
return ck_cmdline;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
2022-05-04 12:36 ` lizhe
@ 2022-05-05 23:19 ` Baoquan He
-1 siblings, 0 replies; 10+ messages in thread
From: Baoquan He @ 2022-05-05 23:19 UTC (permalink / raw)
To: lizhe; +Cc: vgoyal, dyoung, prudo, kexec, linux-kernel
On 05/04/22 at 05:36am, lizhe wrote:
> When ck_cmdline is NULL. The last three lines of
> this function(get_last_crashkernel()) are equivalent to :
> if (!NULL)
> return NULL;
>
> return NULL;
> This is obviously a redundant check
Now the patch log correctly reflects the code change, even though the
log is a little redundant. While it's far far better than a wrong log
which will definitely confuse, even mislead people. I would go with:
======
kernel/crash_core.c : remove redundant check of ck_cmdline
At the end of get_last_crashkernel(), the judgement of ck_cmdline is
obviously unnecessary and causes redundance, let's clean it up.
======
And the patch version is missing. If you agree on the above log
rephrasing, please post v4 with the updated log, and can add my ack:
Acked-by: Baoquan He <bhe@redhat.com>
Thanks
Baoquan
>
> Signed-off-by: lizhe <sensor1010@163.com>
> ---
> kernel/crash_core.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 256cf6db573c..c232f01a2c54 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
> p = strstr(p+1, name);
> }
>
> - if (!ck_cmdline)
> - return NULL;
> -
> return ck_cmdline;
> }
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL
@ 2022-05-05 23:19 ` Baoquan He
0 siblings, 0 replies; 10+ messages in thread
From: Baoquan He @ 2022-05-05 23:19 UTC (permalink / raw)
To: kexec
On 05/04/22 at 05:36am, lizhe wrote:
> When ck_cmdline is NULL. The last three lines of
> this function(get_last_crashkernel()) are equivalent to :
> if (!NULL)
> return NULL;
>
> return NULL;
> This is obviously a redundant check
Now the patch log correctly reflects the code change, even though the
log is a little redundant. While it's far far better than a wrong log
which will definitely confuse, even mislead people. I would go with:
======
kernel/crash_core.c : remove redundant check of ck_cmdline
At the end of get_last_crashkernel(), the judgement of ck_cmdline is
obviously unnecessary and causes redundance, let's clean it up.
======
And the patch version is missing. If you agree on the above log
rephrasing, please post v4 with the updated log, and can add my ack:
Acked-by: Baoquan He <bhe@redhat.com>
Thanks
Baoquan
>
> Signed-off-by: lizhe <sensor1010@163.com>
> ---
> kernel/crash_core.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 256cf6db573c..c232f01a2c54 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -222,9 +222,6 @@ static __init char *get_last_crashkernel(char *cmdline,
> p = strstr(p+1, name);
> }
>
> - if (!ck_cmdline)
> - return NULL;
> -
> return ck_cmdline;
> }
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-05-05 23:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-25 15:38 [PATCH] kernel/crash_core.c : Remove redundant checks for ck_cmdline is NULL lizhe
2022-04-25 15:38 ` lizhe
2022-04-26 8:39 ` Philipp Rudo
2022-04-26 8:39 ` Philipp Rudo
[not found] ` <6a0fa9cc.19d4.1808440ca50.Coremail.sensor1010@163.com>
2022-05-03 16:43 ` Philipp Rudo
2022-05-03 16:43 ` Philipp Rudo
2022-05-04 12:36 lizhe
2022-05-04 12:36 ` lizhe
2022-05-05 23:19 ` Baoquan He
2022-05-05 23:19 ` Baoquan He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.