On Mon, May 26, 2014 at 12:46:09PM +0200, Jes Sorensen wrote:
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> writes:
> > On Sun, May 25, 2014 at 10:43:27PM +0200, Jes.Sorensen@xxxxxxxxxx wrote:
> >> --- a/drivers/staging/rtl8723au/include/rtl8723a_bt_intf.h
> >> +++ b/drivers/staging/rtl8723au/include/rtl8723a_bt_intf.h
> >> @@ -22,6 +22,7 @@
> >> bool rtl8723a_BT_using_antenna_1(struct rtw_adapter *padapter);
> >> bool rtl8723a_BT_enabled(struct rtw_adapter *padapter);
> >> bool rtl8723a_BT_coexist(struct rtw_adapter *padapter);
> >> +void rtl8723a_BT_do_coexist(struct rtw_adapter *padapter);
> >> #else
> >> static inline bool rtl8723a_BT_using_antenna_1(struct rtw_adapter *padapter)
> >> {
> >> @@ -35,6 +36,7 @@ static inline bool rtl8723a_BT_coexist(struct rtw_adapter *padapter)
> >> {
> >> return false;
> >> }
> >> +#define rtl8723a_BT_do_coexist(padapter) do {} while(0)
> >
> > It won't matter in this case, but generally it would be better to make
> > this an empty function so you don't have to worry about different side
> > effects.
> >
> > Several other patches have the same thing.
>
> If you look at the patches, you'll notice that I made them functions
> when there were return values, only the ones returning void were done
> like this. If you can point out an actual problem with this approach,
> please do so.
Jes, does every single review comment have to be a fight? I'm not
asking you to redo things. I've made this same review comment to
several other people this month about adding empty do { } while(0)
macros.
I think you are smart enough and understand about side effects. It's
something that perhaps people worry about too much, you are right. I
have done this research and we only fix maybe three of them per year in
the entire kernel. So they're pretty rare and perhaps not worth the
fuss.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
