Virtualization - May 2022 - [PATCH v3 2/4] vhost-vdpa: introduce STOP backend feature bit

On Fri, May 27, 2022 at 08:50:16AM +0200, Eugenio Perez Martin
wrote:
> On Thu, May 26, 2022 at 9:07 PM Dan Carpenter <dan.carpenter at
oracle.com> wrote:
> >
> > On Thu, May 26, 2022 at 07:00:06PM +0200, Eugenio Perez Martin wrote:
> > > > It feels like returning any literal that isn't 1 or 0
should trigger a
> > > > warning...  I've written that and will check it out
tonight.
> > > >
> > >
> > > I'm not sure this should be so strict, or "literal"
does not include pointers?
> > >
> >
> > What I mean in exact terms, is that if you're returning a known
value
> > and the function returns bool then the known value should be 0 or 1.
> > Don't "return 3;".  This new warning will complain if
you return a known
> > pointer as in "return &a;".  It won't complain if
you return an
> > unknown pointer "return p;".
> >
> 
> Ok, thanks for the clarification.
> 
> > > As an experiment, can Smatch be used to count how many times a
> > > returned pointer is converted to int / bool before returning vs
not
> > > converted?
> >
> > I'm not super excited to write that code...  :/
> >
> 
> Sure, I understand. I meant if it was possible or if that is too far
> beyond its scope.
To be honest, I misread what you were asking.  GCC won't let you return
a pointer with an implied cast to int.  It has to be explicit.  So there
are zero of those.  It's not hard to look for pointers with an implied
cast to bool.

static void match_pointer(struct expression *ret_value)
{
        struct symbol *type;
        char *name;

        type = cur_func_return_type();
        if (!type || sval_type_max(type).value != 1)
                return;

        if (!is_pointer(ret_value))
                return;

        name = expr_to_str(ret_value);
        sm_msg("'%s' return pointer cast to bool", name);
        free_string(name);
}

regards,
dan carpenter

On Fri, May 27, 2022 at 10:36:55AM +0300, Dan Carpenter
wrote:
> static void match_pointer(struct expression *ret_value)
> {
>         struct symbol *type;
>         char *name;
> 
>         type = cur_func_return_type();
>         if (!type || sval_type_max(type).value != 1)
>                 return;
> 
>         if (!is_pointer(ret_value))
>                 return;
> 
>         name = expr_to_str(ret_value);
>         sm_msg("'%s' return pointer cast to bool", name);
>         free_string(name);
> }
I found a bug in Smatch with this check.  With the Smatch bug fixed then
there is only only place which does a legitimate implied pointer to bool
cast.  A different function returns NULL on error instead of false.

drivers/gpu/drm/i915/display/intel_dmc.c:249 intel_dmc_has_payload()
'i915->dmc.dmc_info[0]->payload' return pointer cast to bool
drivers/net/wireless/rndis_wlan.c:1980 rndis_bss_info_update() '(0)'
return pointer cast to bool
drivers/net/wireless/rndis_wlan.c:1989 rndis_bss_info_update() '(0)'
return pointer cast to bool
drivers/net/wireless/rndis_wlan.c:1995 rndis_bss_info_update() '(0)'
return pointer cast to bool

regards,
dan carpenter