Skip to contentSkip to conversation

Staging: rtl8192e: Use struct_size

PATCH Superseded Felix Schlepper 2 participants 5 messages Jun 22, 2022 – Jun 23, 2022 1 message not in the archive Staging

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Felix Schlepper

author
#1 lore raw

PATCH v3 1/3 Staging: rtl8192e: Use struct_size

Using struct_size is encouraged because it is safer than using sizeof and calculations by hand.

Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +1 -2
index 37715afb0210..f2ef32e943ae 100644
@@ -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(struct_size(txb, fragments, nr_frags), gfp_mask);
 	if (!txb)
 		return NULL;
Signature · 1 line
2.36.1
author
#2 lore raw

PATCH v3 2/3 Staging: rtl8192e: Using kzalloc and delete memset

In reply to a message not in the archive

By using kzalloc, we can delete a memset and avoid memory initialization defects.

Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +1 -2
index f2ef32e943ae..1307cf55741a 100644
@@ -205,11 +205,10 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
 	struct rtllib_txb *txb;
 	int i;
 
-	txb = kmalloc(struct_size(txb, fragments, 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);
Signature · 1 line
2.36.1
#3 lore raw

Re: [PATCH v3 2/3] Staging: rtl8192e: Using kzalloc and delete memset

Quoted 3 lines · Felix Schlepper

By using kzalloc, we can delete a memset and avoid memory initialization defects.

This commit message is slightly confusing. The "avoid memory initialization defects" bit is very vague.

The difference between the original code and patched code is that the patched code zeroes out the txb->fragments[] array. The original code worked. The new code is slightly cleaner, but also slower. Whatever, either way is fine.

But just state it clearly in the commit message.

"By using kzalloc, we can delete a memset. The practical difference
is that using kzalloc() will zero out the txb->fragments[] array. The original code worked fine, but zeroing everything seems nicer."

regards,
dan carpenter

author
#4 lore raw

PATCH v3 3/3 Staging: rtl8192e: Cleaning up error handling

In reply to a message not in the archive

This error handling is more readable and reduces code size.

Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +9 -10
index 1307cf55741a..b246f5e0cad7 100644
@@ -214,19 +214,18 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int 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 (unlikely(!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]);
+
+	return NULL;
 }
 
 static int rtllib_classify(struct sk_buff *skb, u8 bIsAmsdu)
Signature · 1 line
2.36.1
#5 lore raw

Re: [PATCH v3 3/3] Staging: rtl8192e: Cleaning up error handling

Quoted 1 line · Felix Schlepper

This error handling is more readable and reduces code size.

There isn't a reduction in code size.

Quoted 24 lines
Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +2 -6
index 1307cf55741a..b246f5e0cad7 100644
@@ -214,19 +214,18 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int 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 (unlikely(!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)

drivers/staging/rtl8192e/rtllib_tx.c static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size, · line 226

Quoted 10 lines
drivers/staging/rtl8192e/rtllib_tx.c +5 -4
-			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]);

There needs to be a kfree(txb);

regards,
dan carpenter

Quoted 4 lines
drivers/staging/rtl8192e/rtllib_tx.c +2 -0
+
+	return NULL;
 }