| Message ID | 20230623144410.1837261-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user: Remove pointless NULL check in clock_adjtime handling | expand |
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>
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~
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; > --
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; > > --
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;
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(-)