From: Dan Carpenter
> Sent: 21 March 2022 09:07
>
> On Sat, Mar 19, 2022 at 10:35:58PM +0000, David Laight wrote:
> > OTOH this code is a big pile of poo.
> > Abstraction functions gone mad.
>
> Yeah.
>
> I wrote an email similar to yours and I even wrote some sample code.
> But then I deleted it because I don't pay Martin anything so I'm just
> grateful for what he sends and can't reasonably ask for more.
>
> This code constantly amazes me with how many abstractions there are.
> Martin keeps deleting them, and I think he's going to run out but so
> far that hasn't happened.
>
> Anyway here is the diff just for laughs since you brought it up. Not
> something that's necessarry and definitely not a priority.
>
> I don't really like enable/disable functions that do opposite things
> depending on if true/false is passed as a parameter. They're normally
> more readable split apart.
>
> Ideally there would be adapter_to_pdbm() and adapter_to_podm() helper
> functions.
>
> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> index 665b077190bc..d973cf341031 100644
> --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> @@ -276,12 +276,23 @@ void Restore_DM_Func_Flag(struct adapter *padapter)
> SetHwReg8188EU(padapter, HW_VAR_DM_FUNC_OP, (u8 *)(&saveflag));
> }
>
> +void enable_dm_func(struct adapter *padapter, u32 mode)
> + {
> + struct dm_pri *pdmpriv = adapter_to_pdbm(padapter);
> + struct odm_dm_struct *podmpriv = adapter_to_pod(padapter);
> +
> + if (mode == DYNAMIC_ALL_FUNC_ENABLE) {
> + podmpriv->SupportAbility = pdmpriv->InitODMFlag;
> + } else {
> + podmpriv->SupportAbility |= mode;
> + }
> +}
> +
> +void disable_dm_func(struct adapter *padapter, u32 mode)
> +{
> + struct odm_dm_struct *podmpriv = adapter_to_pod(padapter);
> +
> + podmpriv->SupportAbility &= mode;
> + }
I'd go even further.
One function for each of 'set', 'enable' and 'disable'.
Doing '=', '|=' and '&= ~'.
But then you realise it just isn't worth the effort.
Just type:
adapter_to_pod(padapter)-> SupportAbility = ...
directly in the code.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
