Skip to contentSkip to conversation

Trial Patch

DISCUSSION nick 7 participants 24 messages Sep 9, 2014 – Sep 10, 2014

lore
Share & tools
lore
#1 lore raw
Quoted 1 line · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix.

Executive summary: Many. Most of which you've been told about before.

Quoted 2 lines

This patch checks in fw_download_code for if the allocated skb is NULl. Further more if the skb is null and we are in the loop,

Make up your mind regarding how many capital letters 'null' has.

This was mentioned to you earlier today. The proper response is *not* to come up with yet another variant.

Quoted 1 line

clean up and dequeune the skb quenue. In additon return false

dequeue, addition.

We do cut some slack for people whose first language isn't English. However, if you're a native speaker, there's really no excuse for not spell-checking the changelog.

Remember - nothing screams poor workmanship quite like wrinkles in the duct tape.

Quoted 3 lines

directly in the if statement and return true by itself removing rt_status to improve the code's readablitiy of return statements in fw_download_code.

Do one thing per patch. Especially when you're still trying to convince us that you're in fact able to do one thing correctly in a patch.

+ skb_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
+ return false;

You have been told before regarding this patch that this bypasses a call to write_nic_byte() at the end of the function. I see zero discussion of why you consider it safe to do so, nor do I see any indication that you've researched whether or not skb_dequeue() is sufficient to deal with cleaning up after a non-initial fragment.

Sorry Nick, but I've pretty much reached the point where any further patch from you that doesn't fully address already-mentioned review comments (including general comments on procedure and the like from *other* patches) are going to be silently tossed into my bit bucket.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/061e2323/attachment.bin

author
#2 lore raw
In reply to nick
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix. Nick

This patch is wrong, checkpatch errors. I am attaching another fixed version.
Sorry Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-staging-Fix-Null-check-for-allocating-skb-in-r8192E_.patch
Type: text/x-patch
Size: 1969 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/157f2a4f/attachment-0001.bin

#3 lore raw
Quoted 14 lines · nick
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix. Nick

This patch is wrong, checkpatch errors. I am attaching another fixed version. Sorry Nick

Quoted 1 line

From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001

From: Nicholas Krause <[email protected]>
Date: Mon, 8 Sep 2014 21:30:37 -0400
Subject: [PATCH] staging: Fix Null check for allocating skb in r8192E_firmware.c

Quoted 7 lines

This patch checks in fw_download_code for if the allocated skb is NULl. Further more if the skb is null and we are in the loop, clean up and dequeune the skb quenue. In additon return false directly in the if statement and return true by itself removing rt_status to improve the code's readablitiy of return statements in fw_download_code.

lots of spelling mistake .. what is NuLl ?

Quoted 18 lines
Signed-off-by Nicholas Krause <[email protected]>
---
 drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +0 -1
index 1a95d1f..6988e1c 100644
@@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 			     u32 buffer_len)
 {
 	struct r8192_priv *priv = rtllib_priv(dev);
-	bool		    rt_status = true;
 	u16		    frag_threshold;
 	u16		    frag_length, frag_offset = 0;
 	int		    i;

drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address, · line 64

Quoted 5 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +2 -0
@@ -61,6 +60,10 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 		}
 
 		skb  = dev_alloc_skb(frag_length + 4);
+		if (skb == NULL) {
+				skb_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);

again checkpatch warning of line over 80 char

Quoted 22 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +3 -1
+				return false;
+		}
 		memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
 		tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
 		tcb_desc->queue_index = TXCMD_QUEUE;

@@ -99,7 +102,7 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 
 	write_nic_byte(dev, TPPoll, TPPoll_CQ);
 
-	return rt_status;
+	return true;
 }
 
 static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
Signature · 7 lines
1.9.1


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
author
#4 lore raw
Quoted 45 lines · Sudip Mukherjee
Quoted 14 lines · nick
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix. Nick

This patch is wrong, checkpatch errors. I am attaching another fixed version. Sorry Nick

Quoted 1 line

From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001

From: Nicholas Krause <[email protected]>
Date: Mon, 8 Sep 2014 21:30:37 -0400
Subject: [PATCH] staging: Fix Null check for allocating skb in r8192E_firmware.c

Quoted 7 lines

This patch checks in fw_download_code for if the allocated skb is NULl. Further more if the skb is null and we are in the loop, clean up and dequeune the skb quenue. In additon return false directly in the if statement and return true by itself removing rt_status to improve the code's readablitiy of return statements in fw_download_code.

lots of spelling mistake .. what is NuLl ?

Quoted 20 lines
Signed-off-by Nicholas Krause <[email protected]>
---
 drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +0 -1
index 1a95d1f..6988e1c 100644
@@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 			     u32 buffer_len)
 {
 	struct r8192_priv *priv = rtllib_priv(dev);
-	bool		    rt_status = true;
 	u16		    frag_threshold;
 	u16		    frag_length, frag_offset = 0;
 	int		    i;

@@ -61,6 +60,10 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 		}
Quoted 29 lines

drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address, · line 64

Quoted 3 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +2 -0
 		skb  = dev_alloc_skb(frag_length + 4);
+		if (skb == NULL) {
+				skb_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);

again checkpatch warning of line over 80 char

Quoted 22 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +3 -1
+				return false;
+		}
 		memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
 		tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
 		tcb_desc->queue_index = TXCMD_QUEUE;

@@ -99,7 +102,7 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 
 	write_nic_byte(dev, TPPoll, TPPoll_CQ);
 
-	return rt_status;
+	return true;
 }
 
 static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
Signature · 7 lines
1.9.1


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Thanks for the feedback, Guys. I will look over this latter and try and fix it. Nick

#5 lore raw
Quoted 72 lines · nick
Quoted 69 lines · Sudip Mukherjee
Quoted 14 lines · nick
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix. Nick

This patch is wrong, checkpatch errors. I am attaching another fixed version. Sorry Nick

Quoted 1 line

From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001

From: Nicholas Krause <[email protected]>
Date: Mon, 8 Sep 2014 21:30:37 -0400
Subject: [PATCH] staging: Fix Null check for allocating skb in r8192E_firmware.c

Quoted 7 lines

This patch checks in fw_download_code for if the allocated skb is NULl. Further more if the skb is null and we are in the loop, clean up and dequeune the skb quenue. In additon return false directly in the if statement and return true by itself removing rt_status to improve the code's readablitiy of return statements in fw_download_code.

lots of spelling mistake .. what is NuLl ?

Quoted 18 lines
Signed-off-by Nicholas Krause <[email protected]>
---
 drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +0 -1
index 1a95d1f..6988e1c 100644
@@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
                          u32 buffer_len)
 {
     struct r8192_priv *priv = rtllib_priv(dev);
-    bool                rt_status = true;
     u16                 frag_threshold;
     u16                 frag_length, frag_offset = 0;
     int                 i;

drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address, · line 64

Quoted 5 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +2 -0
@@ -61,6 +60,10 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
             }
 
             skb  = dev_alloc_skb(frag_length + 4);
+            if (skb == NULL) {
+                            skb_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);

again checkpatch warning of line over 80 char

Quoted 17 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +3 -2
+                            return false;
+            }
             memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
             tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
             tcb_desc->queue_index = TXCMD_QUEUE;

@@ -99,7 +102,7 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 
     write_nic_byte(dev, TPPoll, TPPoll_CQ);
 
-    return rt_status;
+    return true;
 }
 
 static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
--

1.9.1

Quoted 7 lines
Quoted 5 lines
Quoted 4 lines

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Thanks for the feedback, Guys. I will look over this latter and try and fix it. Nick

form the very beginning everyone is asking you to check your patch with checkpatch and build test it before submitting .. but .. :( if you are planning to send another patch, can you please include your checkpatch report in the mail , that will save many people's time over here .

thanks
sudip

#6 lore raw
In reply to nick
Quoted 1 line · nick

This patch is wrong, checkpatch errors. I am attaching another fixed version.

*yawn* You still didn't even fix the typos in comments.

Not Worth The Effort To Review.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/faee2720/attachment.bin

#7 lore raw
In reply to nick
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix.
Nick

Nick, I guess you know this list is a training ground for the main lists. As such the same rules are enforced when it comes to formatting.

Patches as an attachment are simply not acceptable. You have to do them inline. I'm surprised people are even opening your attachments to look at them.

Greg

Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
#8 lore raw
Quoted 10 lines · Greg Freemyer
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix.
Nick

Nick, I guess you know this list is a training ground for the main lists. As such the same rules are enforced when it comes to formatting.

Patches as an attachment are simply not acceptable. You have to do them inline. I'm surprised people are even opening your attachments to look at them.

My understanding is that the problem is not with attachments, but with using strange things like base64 for attachments. Nick has used plain text attachment which seem perfectly fine for me. Check this:

http://www.tux.org/lkml/

"If I get a patch in an attachment (other than a "Text/PLAIN" type attachment with no mangling and that pretty much all mail readers and all tools will see as a normal body), I simply WILL NOT apply it unless I have strong reason to. I usually wont even bother looking at it, unless I expected something special from the sender.

Really. Don't send patches as attachments.

Linus"

So plain text attachment seem to be acceptable...

Quoted 9 lines

Greg
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Signature · 1 line
Peter
author
#9 lore raw
Quoted 43 lines · Peter Senna Tschudin
Quoted 10 lines · Greg Freemyer
Quoted 3 lines · nick

I am attaching a trial patch again , please let me known if there are any issues for me to fix.
Nick

Nick, I guess you know this list is a training ground for the main lists. As such the same rules are enforced when it comes to formatting.

Patches as an attachment are simply not acceptable. You have to do them inline. I'm surprised people are even opening your attachments to look at them.

My understanding is that the problem is not with attachments, but with using strange things like base64 for attachments. Nick has used plain text attachment which seem perfectly fine for me. Check this:

http://www.tux.org/lkml/

"If I get a patch in an attachment (other than a "Text/PLAIN" type attachment with no mangling and that pretty much all mail readers and all tools will see as a normal body), I simply WILL NOT apply it unless I have strong reason to. I usually wont even bother looking at it, unless I expected something special from the sender.

Really. Don't send patches as attachments.

Linus"

So plain text attachment seem to be acceptable...

Quoted 9 lines

Greg
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 2 lines

--
Peter

I understand that , sorry Guys. I am going to fix this later including my spell checks.

author
#11 lore raw
Quoted 7 lines · Valdis.Kletnieks at vt.edu
Quoted 2 lines · Nick Krause

I understand that , sorry Guys. I am going to fix this later including my spell checks.

Don't bother.

Valdis,
I am sending this in again fixed and working. I also looked at your complain about write_nic_byte and this seems to be there if this succeeds.
Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-staging-Fix-Null-check-for-allocating-skb-in-r8192E_.patch
Type: text/x-patch
Size: 1972 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/76055462/attachment.bin

#12 lore raw
Quoted 5 lines · nick
Quoted 2 lines · Valdis.Kletnieks at vt.edu

On Tue, 09 Sep 2014 11:52:20 -0400, Nick Krause said:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

I understand that , sorry Guys. I am going to fix this later including my spell checks.

Don't bother.

Valdis,
I am sending this in again fixed and working. I also looked at your complain about write_nic_byte and this seems to be there if this succeeds.

You've *still* got the spelling errors in there. This is a symptom
of the fundamental reason that your reputation is appalling -- you are sloppy in your work. You miss (or ignore) details, even small easy ones like fixing up the capitalisation of NULL and the spelling on the same line. This sloppiness extends to not understanding the code you're trying to change, not thinking it through, not compiling it before it goes out, not testing it properly before it goes out.

Concentrate on the details. There are lots of details, and you need
to get them *all* right. If someone like Valdis says "you need to fix this, this and this", fix *everything*. Print out the email, use a highlighter pen to indicate everything that needs fixing. Put a tick by every item when you've dealt with it. Don't even *think* about posting a new revision unless you've got a tick next to every single highlighter mark.

If someone asks you a question, like "what happens if the condition
in $line_number is true?", this is intended to make you stop and think. The questions are leading ones, and are trying to make you see for yourself why there is a problem in your code. The way things are at the moment, if you look at one of those questions, and answer it, and the answer *isn't* "<this> bad thing will happen", then you've missed the point of the question, and you need to look at it harder. Not answering the question is not an option at this point.

Hugo.

Quoted 50 lines

Nick

Quoted 1 line

From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001

From: Nicholas Krause <[email protected]>
Date: Mon, 8 Sep 2014 21:30:37 -0400
Subject: [PATCH] staging: Fix Null check for allocating skb in r8192E_firmware.c

This patch checks in fw_download_code for if the allocated skb is NULl. Further more if the skb is null and we are in the loop, clean up and dequeune the skb quenue. In additon return false directly in the if statement and return true by itself removing rt_status to improve the code's readablitiy of return statements in fw_download_code.

Signed-off-by Nicholas Krause <[email protected]>
---
 drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +5 -2
index 1a95d1f..6988e1c 100644
@@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 			     u32 buffer_len)
 {
 	struct r8192_priv *priv = rtllib_priv(dev);
-	bool		    rt_status = true;
 	u16		    frag_threshold;
 	u16		    frag_length, frag_offset = 0;
 	int		    i;

@@ -61,6 +60,10 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 		}
 
 		skb  = dev_alloc_skb(frag_length + 4);
+		if (skb == NULL) {
+				skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
+				return false;
+		}
 		memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
 		tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
 		tcb_desc->queue_index = TXCMD_QUEUE;

@@ -99,7 +102,7 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 
 	write_nic_byte(dev, TPPoll, TPPoll_CQ);
 
-	return rt_status;
+	return true;
 }
 
 static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
Signature · 11 lines
=== Hugo Mills: hugo at ... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
   --- Comic Sans goes into a bar,  and the barman says, "We don't ---   
                         serve your type here."                          
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: Digital signature
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/3dcf5d00/attachment-0001.bin
#13 lore raw
In reply to nick
Quoted 1 line · nick

I am sending this in again fixed and working.

Typos are still there, and zero explanation of how you verified "fixed and working", which given your track record of failing to even compile test your code is a *big* show-stopper.

If you don't care about your patch enough to even bother fixing the typos, why should anybody else care?

Quoted 2 lines

I also looked at your complain about write_nic_byte and this seems to be there if this succeeds.

OK. This is C 101. How does write_nic_byte() get called if we just did the 'return false' and left both the do/while loop and the entire function in your patch?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/67d8e57a/attachment.bin

author
#14 lore raw
Quoted 11 lines · Valdis.Kletnieks at vt.edu
Quoted 1 line · nick

I am sending this in again fixed and working.

Typos are still there, and zero explanation of how you verified "fixed and working", which given your track record of failing to even compile test your code is a *big* show-stopper.

If you don't care about your patch enough to even bother fixing the typos, why should anybody else care?

Quoted 7 lines
Quoted 2 lines

I also looked at your complain about write_nic_byte and this seems to be there if this succeeds.

OK. This is C 101. How does write_nic_byte() get called if we just did the 'return false' and left both the do/while loop and the entire function in your patch?

Not going to execute, I wasn't sure if that statement needs to be executed.
Seems it needs to be for the driver to be activated. Thanks for helping
out with my sloppiness :) and being patience. If there are any more problems
let me known.
Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-staging-Fix-NULL-check-for-allocating-the-skb-in-r81.patch
Type: text/x-patch
Size: 1479 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/b3c63bd9/attachment.bin

#15 lore raw
Quoted 1 line · nick

Not going to execute, I wasn't sure if that statement needs to be executed.

skb = dev_alloc_skb(frag_length + 4);
+ if (skb == NULL) {
+ skb_queue_purge(&priv->rtllib->skbwaitQ[TXCMD_QUEUE];
+ break;
+ }

OK. It's official. I see *zero* possibility that you'll ever manage to write an acceptable patch. This is *still* wrong.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/d3ba2201/attachment-0001.bin

author
#16 lore raw
Quoted 9 lines · Valdis.Kletnieks at vt.edu
Quoted 1 line · nick

Not going to execute, I wasn't sure if that statement needs to be executed.

(quoted diff) +4 -0
 		skb  = dev_alloc_skb(frag_length + 4);
+		if (skb == NULL) {
+			skb_queue_purge(&priv->rtllib->skbwaitQ[TXCMD_QUEUE];
+			break;
+		}
Quoted 3 lines

OK. It's official. I see *zero* possibility that you'll ever manage to write an acceptable patch. This is *still* wrong.

Care to explain.
Nick

#17 lore raw
Quoted 4 lines · nick
Quoted 3 lines

OK. It's official. I see *zero* possibility that you'll ever manage to write an acceptable patch. This is *still* wrong.

Care to explain.

No. Please go figure it out for yourself.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/9ad3bb69/attachment.bin

author
#18 lore raw
Quoted 9 lines · Valdis.Kletnieks at vt.edu
Quoted 4 lines · nick
Quoted 3 lines

OK. It's official. I see *zero* possibility that you'll ever manage to write an acceptable patch. This is *still* wrong.

Care to explain.

No. Please go figure it out for yourself.

I think I fixed it through. If not I known why.
Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-staging-Fix-NULL-check-for-allocating-the-skb-in-r81.patch
Type: text/x-patch
Size: 1502 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/db832b4b/attachment.bin

author
#19 lore raw
Quoted 8 lines · nick
Quoted 5 lines · Valdis.Kletnieks at vt.edu
Quoted 3 lines · nick
Quoted 3 lines

OK. It's official. I see *zero* possibility that you'll ever manage to write an acceptable patch. This is *still* wrong.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Care to explain.

No. Please go figure it out for yourself.

I think I fixed it through. If not I known why.
Nick

Sorry Valdis,
Build Error. Fixed it. I need to really check my patches first :(.
In addition, I want you to forget about and not reply to any patches
you known I haven't build tested.
Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-staging-Fix-NULL-check-for-allocating-the-skb-in-r81.patch
Type: text/x-patch
Size: 1481 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140909/4072089e/attachment-0001.bin

#20 lore raw
Quoted 1 line · nick

Build Error. Fixed it. I need to really check my patches first :(.

How many times have you done that now?

Quoted 2 lines

In addition, I want you to forget about and not reply to any patches you known I haven't build tested.

No, it doesn't work like that. Each broken post counts against you.

Review

Quoted 4 lines
(quoted diff) +4 -0
+		if (skb == NULL) {
+			skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
+			break;
+		}

Like this one - it's *still* wrong. Because....

From: Nicholas Krause <[email protected]>
Date: Tue, 9 Sep 2014 21:12:56 -0400

You reposted the same broken patch again.

Please do the Linux kernel community a favor and seek gainful employment in the food service industry or similar, where your skills will be more appreciated.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140910/d92401d9/attachment.bin

author
#21 lore raw
Quoted 17 lines · Valdis.Kletnieks at vt.edu
Quoted 1 line · nick

Build Error. Fixed it. I need to really check my patches first :(.

How many times have you done that now?

Quoted 2 lines

In addition, I want you to forget about and not reply to any patches you known I haven't build tested.

No, it doesn't work like that. Each broken post counts against you.

Review

Quoted 4 lines
(quoted diff) +4 -0
+		if (skb == NULL) {
+			skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
+			break;
+		}

Like this one - it's *still* wrong. Because....

Quoted 9 lines

From: Nicholas Krause <[email protected]>
Date: Tue, 9 Sep 2014 21:12:56 -0400

You reposted the same broken patch again.

Please do the Linux kernel community a favor and seek gainful employment in the food service industry or similar, where your skills will be more appreciated.

Here is the correct patch.
Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-staging-Fix-NULL-check-for-allocating-the-skb-in-r81.patch
Type: text/x-patch
Size: 1481 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140910/a09e101e/attachment.bin

#22 lore raw
Quoted 1 line · nick

On 14-09-10 12:49 AM, Valdis.Kletnieks at vt.edu wrote:

[snip]

Quoted 6 lines
Quoted 6 lines

Like this one - it's *still* wrong. Because....

From: Nicholas Krause <[email protected]>
Date: Tue, 9 Sep 2014 21:12:56 -0400

You reposted the same broken patch again.

[snip]

Quoted 27 lines

Here is the correct patch.
Nick

Quoted 1 line

From ad55b627241203a380dd9c708a4feae98446f85f Mon Sep 17 00:00:00 2001

From: Nicholas Krause <[email protected]>
Date: Tue, 9 Sep 2014 21:12:56 -0400
Subject: [PATCH] staging: Fix NULL check for allocating the skb in
r8192E_firmware.c

This patch checks in the loop of the function, fw_download_code if a allocated skb is NULL. If the skb is NULL then we first free the allocated skbs in the queue of this function with skb_queue_purge in order to free the allocated skbs. After the freeing of the queue we break out of the do/while of this function as we need to run write_nic_byte in order to activate parts of the network card's driver.

Signed-off-by Nicholas Krause <[email protected]>
---
 drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +0 -0
index 1a95d1f..a251414 100644

drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address, · line 70

Quoted 10 lines
drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c +4 -0
@@ -61,6 +61,10 @@ static bool fw_download_code(struct net_device *dev, u8 *code_virtual_address,
 		}
 
 		skb  = dev_alloc_skb(frag_length + 4);
+		if (skb == NULL) {
+			skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
+			break;
+		}
 		memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
 		tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
 		tcb_desc->queue_index = TXCMD_QUEUE;

I'll save Valdis the trouble here. This is still wrong. It's
*identical* to the last one you sent. Bit for bit. Again, you've lost track of even the most fundamental details ("Am I sending the thing I wanted to?")

Doing this once or twice, occasionally, is OK. Everyone makes
mistakes every so often. Doing it repeatedly, on a nearly daily basis, over a period of a couple of months, is simply unacceptable.

Hugo.

Signature · 12 lines
=== Hugo Mills: hugo at ... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
   --- Our so-called leaders speak/with words they try to jail ya/ ---   
        They subjugate the meek/but it's the rhetoric of failure.        
                                                                         
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: Digital signature
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140910/1f99dd13/attachment.bin
#23 lore raw
In reply to nick

This is exactly the same patch as you sent last time. And it's still wrong.

Nick, just out of curiosity: How old are you?

Quoted 37 lines · nick
Quoted 17 lines · Valdis.Kletnieks at vt.edu
Quoted 1 line · nick

Build Error. Fixed it. I need to really check my patches first :(.

How many times have you done that now?

Quoted 2 lines

In addition, I want you to forget about and not reply to any patches you known I haven't build tested.

No, it doesn't work like that. Each broken post counts against you.

Review

Quoted 4 lines
(quoted diff) +4 -0
+            if (skb == NULL) {
+                    skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
+                    break;
+            }

Like this one - it's *still* wrong. Because....

Quoted 9 lines

From: Nicholas Krause <[email protected]>
Date: Tue, 9 Sep 2014 21:12:56 -0400

You reposted the same broken patch again.

Please do the Linux kernel community a favor and seek gainful employment in the food service industry or similar, where your skills will be more appreciated.

Here is the correct patch.
Nick

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies