1. Patchew
  2. QEMU
  3. View series

[PATCH] linux-user: Remove pointless NULL check in clock_adjtime handling

Peter Maydell posted 1 patch 2 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230623144410.1837261-1-peter.maydell@linaro.org
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/syscall.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
[PATCH] linux-user: Remove pointless NULL check in clock_adjtime handling
Posted by Peter Maydell 2 years, 5 months ago 
In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to
the address of the local variable htx.  This means it can never be
NULL, but later in the code we check it for NULL anyway.  Coverity
complains about this (CID 1507683) because the NULL check comes after
a call to clock_adjtime() that assumes it is non-NULL.

Since phtx is always &htx, and is used only in three places, it's not
really necessary.  Remove it, bringing the code structure in to line
with that for TARGET_NR_clock_adjtime64, which already uses a simple
'&htx' when it wants a pointer to 'htx'.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f2cb101d83c..7b2f9f7340e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10935,16 +10935,14 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
 #if defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME)
     case TARGET_NR_clock_adjtime:
         {
-            struct timex htx, *phtx = &htx;
+            struct timex htx;
 
-            if (target_to_host_timex(phtx, arg2) != 0) {
+            if (target_to_host_timex(&htx, arg2) != 0) {
                 return -TARGET_EFAULT;
             }
-            ret = get_errno(clock_adjtime(arg1, phtx));
-            if (!is_error(ret) && phtx) {
-                if (host_to_target_timex(arg2, phtx) != 0) {
-                    return -TARGET_EFAULT;
-                }
+            ret = get_errno(clock_adjtime(arg1, &htx));
+            if (!is_error(ret) && host_to_target_timex(arg2, &htx)) {
+                return -TARGET_EFAULT;
             }
         }
         return ret;
-- 
2.34.1
Re: [PATCH] linux-user: Remove pointless NULL check in clock_adjtime handling
Posted by Peter Maydell 2 years, 5 months ago 
Laurent, ping? This patch has been reviewed.

thanks
-- PMM

On Fri, 23 Jun 2023 at 15:44, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to
> the address of the local variable htx.  This means it can never be
> NULL, but later in the code we check it for NULL anyway.  Coverity
> complains about this (CID 1507683) because the NULL check comes after
> a call to clock_adjtime() that assumes it is non-NULL.
>
> Since phtx is always &htx, and is used only in three places, it's not
> really necessary.  Remove it, bringing the code structure in to line
> with that for TARGET_NR_clock_adjtime64, which already uses a simple
> '&htx' when it wants a pointer to 'htx'.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f2cb101d83c..7b2f9f7340e 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10935,16 +10935,14 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>  #if defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME)
>      case TARGET_NR_clock_adjtime:
>          {
> -            struct timex htx, *phtx = &htx;
> +            struct timex htx;
>
> -            if (target_to_host_timex(phtx, arg2) != 0) {
> +            if (target_to_host_timex(&htx, arg2) != 0) {
>                  return -TARGET_EFAULT;
>              }
> -            ret = get_errno(clock_adjtime(arg1, phtx));
> -            if (!is_error(ret) && phtx) {
> -                if (host_to_target_timex(arg2, phtx) != 0) {
> -                    return -TARGET_EFAULT;
> -                }
> +            ret = get_errno(clock_adjtime(arg1, &htx));
> +            if (!is_error(ret) && host_to_target_timex(arg2, &htx)) {
> +                return -TARGET_EFAULT;
>              }
>          }
>          return ret;
> --
Re: [PATCH] linux-user: Remove pointless NULL check in clock_adjtime handling
Posted by Peter Maydell 2 years, 5 months ago 
I'll take this via target-arm.next unless there are any
objections...

thanks
-- PMM

On Tue, 4 Jul 2023 at 14:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Laurent, ping? This patch has been reviewed.
>
> thanks
> -- PMM
>
> On Fri, 23 Jun 2023 at 15:44, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to
> > the address of the local variable htx.  This means it can never be
> > NULL, but later in the code we check it for NULL anyway.  Coverity
> > complains about this (CID 1507683) because the NULL check comes after
> > a call to clock_adjtime() that assumes it is non-NULL.
> >
> > Since phtx is always &htx, and is used only in three places, it's not
> > really necessary.  Remove it, bringing the code structure in to line
> > with that for TARGET_NR_clock_adjtime64, which already uses a simple
> > '&htx' when it wants a pointer to 'htx'.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  linux-user/syscall.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index f2cb101d83c..7b2f9f7340e 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -10935,16 +10935,14 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
> >  #if defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME)
> >      case TARGET_NR_clock_adjtime:
> >          {
> > -            struct timex htx, *phtx = &htx;
> > +            struct timex htx;
> >
> > -            if (target_to_host_timex(phtx, arg2) != 0) {
> > +            if (target_to_host_timex(&htx, arg2) != 0) {
> >                  return -TARGET_EFAULT;
> >              }
> > -            ret = get_errno(clock_adjtime(arg1, phtx));
> > -            if (!is_error(ret) && phtx) {
> > -                if (host_to_target_timex(arg2, phtx) != 0) {
> > -                    return -TARGET_EFAULT;
> > -                }
> > +            ret = get_errno(clock_adjtime(arg1, &htx));
> > +            if (!is_error(ret) && host_to_target_timex(arg2, &htx)) {
> > +                return -TARGET_EFAULT;
> >              }
> >          }
> >          return ret;
> > --
Re: [PATCH] linux-user: Remove pointless NULL check in clock_adjtime handling
Posted by Richard Henderson 2 years, 5 months ago 
On 6/23/23 16:44, Peter Maydell wrote:
> In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to
> the address of the local variable htx.  This means it can never be
> NULL, but later in the code we check it for NULL anyway.  Coverity
> complains about this (CID 1507683) because the NULL check comes after
> a call to clock_adjtime() that assumes it is non-NULL.
> 
> Since phtx is always &htx, and is used only in three places, it's not
> really necessary.  Remove it, bringing the code structure in to line
> with that for TARGET_NR_clock_adjtime64, which already uses a simple
> '&htx' when it wants a pointer to 'htx'.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/syscall.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Re: [PATCH] linux-user: Remove pointless NULL check in clock_adjtime handling
Posted by Philippe Mathieu-Daudé 2 years, 5 months ago 
On 23/6/23 16:44, Peter Maydell wrote:
> In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to
> the address of the local variable htx.  This means it can never be
> NULL, but later in the code we check it for NULL anyway.  Coverity
> complains about this (CID 1507683) because the NULL check comes after
> a call to clock_adjtime() that assumes it is non-NULL.
> 
> Since phtx is always &htx, and is used only in three places, it's not
> really necessary.  Remove it, bringing the code structure in to line
> with that for TARGET_NR_clock_adjtime64, which already uses a simple
> '&htx' when it wants a pointer to 'htx'.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/syscall.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.