Re: [PATCH] staging: ks7010: pass 'int' instead of 'bool' to 'hostif_mib_set_request_bool'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 12, 2018 at 08:20:16PM +0200, Sergio Paracuellos wrote:
> On Thu, Jul 12, 2018 at 2:13 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> > On Thu, Jul 12, 2018 at 06:57:42AM +0200, Sergio Paracuellos wrote:
> >> 'hostif_mib_set_request_bool' function receives a bool as value and
> >> send the received value with MIB_VALUE_TYPE_BOOL type. There is
> >> one case where the value passed is not a boolean one but
> >> 'MCAST_FILTER_PROMISC' which is '2'. Pass 'int' instead to avoid
> >> the problem.
> >>
> >> Fixes: 8ce76bff0e6a ("staging: ks7010: add new helpers to achieve
> >> mib set request and simplify code")
> >>
> >> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> >> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> >> ---
> >>  drivers/staging/ks7010/ks_hostif.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
> >> index 0ecffab..6582566 100644
> >> --- a/drivers/staging/ks7010/ks_hostif.c
> >> +++ b/drivers/staging/ks7010/ks_hostif.c
> >> @@ -1228,7 +1228,7 @@ static inline void hostif_mib_set_request_int(struct ks_wlan_private *priv,
> >>
> >>  static inline void hostif_mib_set_request_bool(struct ks_wlan_private *priv,
> >>                                              enum mib_attribute attr,
> >> -                                            bool val)
> >> +                                            int val)
> >
> > Huh...  This doesn't feel like the right thing.  I thought we should
> > change the callers to use hostif_mib_set_request_int() instead.
> 
> Yes, I though to call that instead at first moment but I end up in revert to the
> previous behaviour...

It's a tricky thing...

The choices are:

1) Fix it in the CorrectWay[tm] which we both agree is
   hostif_mib_set_request_int()

2) Revert to something that looks buggy.  But there is a chance it has
   been tested and works.  We would hide the static checker warning
   which would make the bug harder to fix in the future.

3) Leave the code as-is and wait until someone can test it.  At least
   the static checker warning is there so we will fix it eventually.

I feel like we should take option 1 and if no one complains that means
either no one is using the driver or it works.  Long term that's the
best option.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux