Messages in this thread | | | From | "Fabio M. De Francesco" <> | Subject | Re: [PATCH v4 1/7] staging: r8188eu: remove unnecessary braces in single statement block | Date | Fri, 22 Apr 2022 01:20:29 +0200 |
| |
On giovedì 21 aprile 2022 21:52:14 CEST Fabio M. De Francesco wrote: > On giovedì 21 aprile 2022 21:20:26 CEST Jaehee Park wrote: > > Remove braces for single statement block to minimize the number of > > empty lines, without loss of readability. Issue found with checkpatch. > > WARNING: braces {} are not necessary for single statement blocks > > > > Signed-off-by: Jaehee Park <jhpark1013@gmail.com> > > --- > > drivers/staging/r8188eu/core/rtw_mlme.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/ > r8188eu/core/rtw_mlme.c > > index 3e9882f89f76..d3f4d7cdfa08 100644 > > --- a/drivers/staging/r8188eu/core/rtw_mlme.c > > +++ b/drivers/staging/r8188eu/core/rtw_mlme.c > > @@ -112,9 +112,8 @@ void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv) > > > > rtw_free_mlme_priv_ie_data(pmlmepriv); > > > > - if (pmlmepriv) { > > + if (pmlmepriv) > > vfree(pmlmepriv->free_bss_buf); > > - } > > > > } > > > > -- > > 2.25.1 > > > > Hi Jaehee, > > If you just want to remove the braces, your patch is good. > > But... Did you ask yourself why this code needs to test if "pmlmepriv" is > valid before calling vfree() on it? Actually, the code doesn't need this > test, it just needs to call "vfree(pmlmepriv)" unconditionally.
When copy-pasting I overlooked a couple of things... However the argument is the same: the code doesn't need that 'if' statement before vfree().
I wanted to write "[] it just needs to call "vfree(pmlmepriv- >free_bss_buf)" unconditionally".
I apologize for not reading twice before sending my email too soon after writing and moving sentences around.
> OK, I know that this is not directly related to your patch or any other > that is explicitly made just for removing unnecessary braces...
"pmlmepriv" is valid because we know it from the previous call to rtw_free_mlme_priv_ie_data(). Therefore the code can dereference "pmlmepriv".
Furthermore, even if "free_bss_buf" were NULL it wouldn't matter because "vfree(pmlmepriv->free_bss_buf)" would be a no-op.
Sorry if this inaccurate wording might have confused you about what I was trying to say.
Thanks,
Fabio
| |