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

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

[Peter Maydell]

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

[Philippe Mathieu-Daudé]

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>

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

[Richard Henderson]

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

[Peter Maydell]

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

[Peter Maydell]

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;
> > --