Re: [Qemu-trivial] [Qemu-devel] [PATCH] syscall: fix compiler warnings (clang 5)

[Philippe Mathieu-Daudé]

On 05/10/2017 09:32 AM, Laurent Vivier wrote:
> On 06/05/2017 02:41, Philippe Mathieu-Daudé wrote:
> > static code analyzer complain:
> >
> > linux-user/syscall.c:5575:9: warning: Dereference of undefined pointer value
> >     if (*host_rt_dev_ptr != 0) {
> >         ^~~~~~~~~~~~~~~~
> >
> > Reported-by: Clang Static Analyzer
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> >  linux-user/syscall.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index cec8428589..c541e22693 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -5526,7 +5526,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t 
> > *buf_temp,
> >      int target_size;
> >      void *argptr;
> >      abi_ulong *target_rt_dev_ptr;
> > -    unsigned long *host_rt_dev_ptr;
> > +    unsigned long *host_rt_dev_ptr = NULL;
> >      abi_long ret;
> >      int i;
> >
> > @@ -5572,7 +5572,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t 
> > *buf_temp,
> >      unlock_user(argptr, arg, 0);
> >
> >      ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
>
> I think we should "assert(host_rt_dev_ptr)" here. It's a bug if
> host_rt_dev_ptr is not set.
>
> The "for" loop scans the structure to find the rt_dev field, and we
> should always enter in the first "if", so "host_rt_dev_ptr" is always set.

It's better indeed, I'll use g_assert_nonnull() and resend.

Thanks!

> > -    if (*host_rt_dev_ptr != 0) {
> > +    if (host_rt_dev_ptr != NULL && *host_rt_dev_ptr != 0) {
> >          unlock_user((void *)*host_rt_dev_ptr,
> >                      *target_rt_dev_ptr, 0);
> >      }
>
> Laurent