Re: [Outreachy kernel] [PATCH v2] staging: rtl8192u: Use kmalloc_array instead of kmalloc

[Julia Lawall <julia.lawall@lip6.fr>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4058 bytes --]



On Tue, 9 Feb 2016, Janani Ravichandran wrote:

> 
> 
> On Tuesday, 9 February 2016 17:01:18 UTC-5, Julia Lawall wrote:
> 
> 
>       On Tue, 9 Feb 2016, Janani Ravichandran wrote:
> 
>       > Use kmalloc_array instead of kmalloc for arrays. Coccinelle
>       patch used:
>       >
>       > @@
>       > expression E, F;
>       > constant FLAG;
>       > @@
>       >
>       > - kmalloc(E*F, FLAG)
>       > + kmalloc_array(E, F, FLAG)
> 
>       Actually, your patch doesn't do exactly this.  Your patch
>       (correctly) has
>       E and F switched in the call to kmalloc_array.
> 
> I actually used the semantic patch to find instances where the conversion
> could be applied,
> then looked at the declaration of kmalloc_array to determine the correct
> order of
> arguments and did the change manually to make sure the number of elements
> was the
> first argument and the size of the element was the second argument.
> 
> Please let me know if I should change anything here. Thank you!

The best solution would be to put:

- kmalloc(sizeof(T)*F, FLAG)
+ kmalloc_array(F, sizeof(T), FLAG)

where T is declared as a type metavariable.  This way one can see 
immediately that you have thought about the order in which to put the 
arguments.

julia

> 
> Janani
> 
>       >
>       > Signed-off-by: Janani Ravichandran <janani....@gmail.com>
>       > ---
>       > Changes since v1:
>       >  * Corrected an error in commit message.
> 
>       "an error" is not very explicit.
> 
>       julia
> 
>       >  drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 4 ++--
>       >  1 file changed, 2 insertions(+), 2 deletions(-)
>       >
>       > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
>       b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
>       > index 28ad63a..fb344d1 100644
>       > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
>       > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
>       > @@ -601,8 +601,8 @@ static void RxReorderIndicatePacket(struct
>       ieee80211_device *ieee,
>       >          bool                        bMatchWinStart = false,
>       bPktInBuf = false;
>       >          IEEE80211_DEBUG(IEEE80211_DL_REORDER,"%s(): Seq is
>       %d,pTS->RxIndicateSeq is %d, WinSize is
>       %d\n",__func__,SeqNum,pTS->RxIndicateSeq,WinSize);
>       >  
>       > -        prxbIndicateArray = kmalloc(sizeof(struct
>       ieee80211_rxb *) *
>       > -                        REORDER_WIN_SIZE, GFP_KERNEL);
>       > +        prxbIndicateArray = kmalloc_array(RECORDER_WIN_SIZE,
>       > +                        sizeof(struct ieee80211_rxb *),
>       GFP_KERNEL);
>       >          if (!prxbIndicateArray)
>       >                  return;
>       >  
>       > --
>       > 2.5.0
>       >
>       > --
>       > You received this message because you are subscribed to the
>       Google Groups "outreachy-kernel" group.
>       > To unsubscribe from this group and stop receiving emails from
>       it, send an email to outreachy-kern...@googlegroups.com.
>       > To post to this group, send email to
>       outreach...@googlegroups.com.
>       > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20160210013122.GA12148%4
>       0janani-Inspiron-3521.
>       > For more options, visit https://groups.google.com/d/optout.
>       >
> 
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/37a3f0ea-26e5-4d78-8730-
> 81b7ed756644%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
> 
>