* Re: [PATCH] Staging: rtl8192e: make sizeof type-independent
2022-06-21 14:58 [PATCH] Staging: rtl8192e: make sizeof type-independent Felix Schlepper
@ 2022-06-21 15:02 ` Dan Carpenter
2022-06-21 15:25 ` Joe Perches
1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2022-06-21 15:02 UTC (permalink / raw)
To: Felix Schlepper; +Cc: gregkh, linux-staging, linux-kernel
On Tue, Jun 21, 2022 at 04:58:45PM +0200, Felix Schlepper wrote:
> Making sizeof operator type-independent.
>
> Reported by checkpatch:
>
> CHECK: Prefer kmalloc(sizeof(*txb)...) over
> kmalloc(sizeof(struct rtllib_txb)...)
>
> ---
> Note: First patch, trying to follow kernelnewbies tutorial.
>
> Signed-off-by: Felix Schlepper <f3sch.git@outlook.com>
> ---
> drivers/staging/rtl8192e/rtllib_tx.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtllib_tx.c b/drivers/staging/rtl8192e/rtllib_tx.c
> index 37715afb0210..01e163e61f0e 100644
> --- a/drivers/staging/rtl8192e/rtllib_tx.c
> +++ b/drivers/staging/rtl8192e/rtllib_tx.c
> @@ -205,8 +205,7 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
> struct rtllib_txb *txb;
> int i;
>
> - txb = kmalloc(sizeof(struct rtllib_txb) + (sizeof(u8 *) * nr_frags),
> - gfp_mask);
> + txb = kmalloc(sizeof(*txb) + (sizeof(u8 *) * nr_frags), gfp_mask);
Use the struct_size() macro instead.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Staging: rtl8192e: make sizeof type-independent
2022-06-21 14:58 [PATCH] Staging: rtl8192e: make sizeof type-independent Felix Schlepper
2022-06-21 15:02 ` Dan Carpenter
@ 2022-06-21 15:25 ` Joe Perches
1 sibling, 0 replies; 3+ messages in thread
From: Joe Perches @ 2022-06-21 15:25 UTC (permalink / raw)
To: Felix Schlepper, gregkh; +Cc: linux-staging, linux-kernel
On Tue, 2022-06-21 at 16:58 +0200, Felix Schlepper wrote:
> Making sizeof operator type-independent.
>
> Reported by checkpatch:
>
> CHECK: Prefer kmalloc(sizeof(*txb)...) over
> kmalloc(sizeof(struct rtllib_txb)...)
>
> ---
> Note: First patch, trying to follow kernelnewbies tutorial.
Congrats on the first patch, but know that checkpatch is a stupid
script and it doesn't necessarily offer the best advice.
A helper mechanism exists that should be preferred over checkpatch's
advice...
> diff --git a/drivers/staging/rtl8192e/rtllib_tx.c b/drivers/staging/rtl8192e/rtllib_tx.c
[]
> @@ -205,8 +205,7 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
> struct rtllib_txb *txb;
> int i;
>
> - txb = kmalloc(sizeof(struct rtllib_txb) + (sizeof(u8 *) * nr_frags),
> - gfp_mask);
> + txb = kmalloc(sizeof(*txb) + (sizeof(u8 *) * nr_frags), gfp_mask);
Use struct_size() instead, something like:
txb = kmalloc(struct_size(txb, fragments, nr_frags), gfp_mask);
> if (!txb)
> return NULL;
>
though I would also suggest using kzalloc as safer against memory
initialization defects (and could remove a memset here).
Perhaps something like:
---
drivers/staging/rtl8192e/rtllib_tx.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_tx.c b/drivers/staging/rtl8192e/rtllib_tx.c
index 37715afb0210d..bcccde91fa0bd 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -205,30 +205,28 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
struct rtllib_txb *txb;
int i;
- txb = kmalloc(sizeof(struct rtllib_txb) + (sizeof(u8 *) * nr_frags),
- gfp_mask);
+ txb = kzalloc(struct_size(txb, fragments, nr_frags), gfp_mask);
if (!txb)
return NULL;
- memset(txb, 0, sizeof(struct rtllib_txb));
txb->nr_frags = nr_frags;
txb->frag_size = cpu_to_le16(txb_size);
for (i = 0; i < nr_frags; i++) {
txb->fragments[i] = dev_alloc_skb(txb_size);
- if (unlikely(!txb->fragments[i])) {
- i--;
- break;
- }
+ if (!txb->fragments[i])
+ goto err_free;
memset(txb->fragments[i]->cb, 0, sizeof(txb->fragments[i]->cb));
}
- if (unlikely(i != nr_frags)) {
- while (i >= 0)
- dev_kfree_skb_any(txb->fragments[i--]);
- kfree(txb);
- return NULL;
- }
+
return txb;
+
+err_free:
+ while (i > 0)
+ dev_kfree_skb_any(txb->fragments[--i]);
+ kfree(txb);
+
+ return NULL;
}
static int rtllib_classify(struct sk_buff *skb, u8 bIsAmsdu)
^ permalink raw reply [flat|nested] 3+ messages in thread