* Trial Patch
@ 2014-09-09 3:08 nick
2014-09-09 4:04 ` Valdis.Kletnieks at vt.edu
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: nick @ 2014-09-09 3:08 UTC (permalink / raw)
To: kernelnewbies
I am attaching a trial patch again , please let me known if there are any issues for me to fix.
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: 1915 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140908/5bbf1bed/attachment.bin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 3:08 Trial Patch nick
@ 2014-09-09 4:04 ` Valdis.Kletnieks at vt.edu
2014-09-09 12:22 ` nick
2014-09-09 13:45 ` Greg Freemyer
2 siblings, 0 replies; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-09 4:04 UTC (permalink / raw)
To: kernelnewbies
On Mon, 08 Sep 2014 23:08:46 -0400, nick said:
> 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.
> 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.
> 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.
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 3:08 Trial Patch nick
2014-09-09 4:04 ` Valdis.Kletnieks at vt.edu
@ 2014-09-09 12:22 ` nick
2014-09-09 12:42 ` Sudip Mukherjee
2014-09-09 13:26 ` Valdis.Kletnieks at vt.edu
2014-09-09 13:45 ` Greg Freemyer
2 siblings, 2 replies; 24+ messages in thread
From: nick @ 2014-09-09 12:22 UTC (permalink / raw)
To: kernelnewbies
On 14-09-08 11:08 PM, nick wrote:
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 12:22 ` nick
@ 2014-09-09 12:42 ` Sudip Mukherjee
2014-09-09 13:24 ` nick
2014-09-09 13:26 ` Valdis.Kletnieks at vt.edu
1 sibling, 1 reply; 24+ messages in thread
From: Sudip Mukherjee @ 2014-09-09 12:42 UTC (permalink / raw)
To: kernelnewbies
On Tue, Sep 09, 2014 at 08:22:59AM -0400, nick wrote:
>
>
> On 14-09-08 11:08 PM, nick wrote:
> > 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
> >From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001
> From: Nicholas Krause <xerofoify@gmail.com>
> 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.
>
lots of spelling mistake .. what is NuLl ?
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..6988e1c 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -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_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
again checkpatch warning of line over 80 char
> + 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
>
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies at kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 12:42 ` Sudip Mukherjee
@ 2014-09-09 13:24 ` nick
2014-09-09 13:39 ` Sudip Mukherjee
0 siblings, 1 reply; 24+ messages in thread
From: nick @ 2014-09-09 13:24 UTC (permalink / raw)
To: kernelnewbies
On 14-09-09 08:42 AM, Sudip Mukherjee wrote:
> On Tue, Sep 09, 2014 at 08:22:59AM -0400, nick wrote:
>>
>>
>> On 14-09-08 11:08 PM, nick wrote:
>>> 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
>
>> >From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001
>> From: Nicholas Krause <xerofoify@gmail.com>
>> 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.
>>
>
> lots of spelling mistake .. what is NuLl ?
>
>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
>> ---
>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> index 1a95d1f..6988e1c 100644
>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> @@ -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_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
>
> again checkpatch warning of line over 80 char
>
>> + 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
>>
>
>> _______________________________________________
>> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 12:22 ` nick
2014-09-09 12:42 ` Sudip Mukherjee
@ 2014-09-09 13:26 ` Valdis.Kletnieks at vt.edu
1 sibling, 0 replies; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-09 13:26 UTC (permalink / raw)
To: kernelnewbies
On Tue, 09 Sep 2014 08:22:59 -0400, nick said:
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 13:24 ` nick
@ 2014-09-09 13:39 ` Sudip Mukherjee
0 siblings, 0 replies; 24+ messages in thread
From: Sudip Mukherjee @ 2014-09-09 13:39 UTC (permalink / raw)
To: kernelnewbies
On Tue, Sep 9, 2014 at 6:54 PM, nick <xerofoify@gmail.com> wrote:
>
>
> On 14-09-09 08:42 AM, Sudip Mukherjee wrote:
>> On Tue, Sep 09, 2014 at 08:22:59AM -0400, nick wrote:
>>>
>>>
>>> On 14-09-08 11:08 PM, nick wrote:
>>>> 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
>>
>>> >From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001
>>> From: Nicholas Krause <xerofoify@gmail.com>
>>> 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.
>>>
>>
>> lots of spelling mistake .. what is NuLl ?
>>
>>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
>>> ---
>>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> index 1a95d1f..6988e1c 100644
>>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> @@ -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_dequeue(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
>>
>> again checkpatch warning of line over 80 char
>>
>>> + 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
>>>
>>
>>> _______________________________________________
>>> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 3:08 Trial Patch nick
2014-09-09 4:04 ` Valdis.Kletnieks at vt.edu
2014-09-09 12:22 ` nick
@ 2014-09-09 13:45 ` Greg Freemyer
2014-09-09 13:54 ` Peter Senna Tschudin
2 siblings, 1 reply; 24+ messages in thread
From: Greg Freemyer @ 2014-09-09 13:45 UTC (permalink / raw)
To: kernelnewbies
On September 8, 2014 11:08:46 PM EDT, nick <xerofoify@gmail.com> wrote:
>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
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 13:45 ` Greg Freemyer
@ 2014-09-09 13:54 ` Peter Senna Tschudin
2014-09-09 15:52 ` Nick Krause
0 siblings, 1 reply; 24+ messages in thread
From: Peter Senna Tschudin @ 2014-09-09 13:54 UTC (permalink / raw)
To: kernelnewbies
On Tue, Sep 9, 2014 at 3:45 PM, Greg Freemyer <greg.freemyer@gmail.com> wrote:
>
>
> On September 8, 2014 11:08:46 PM EDT, nick <xerofoify@gmail.com> wrote:
>>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...
>
> 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
--
Peter
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 13:54 ` Peter Senna Tschudin
@ 2014-09-09 15:52 ` Nick Krause
2014-09-09 16:40 ` Valdis.Kletnieks at vt.edu
0 siblings, 1 reply; 24+ messages in thread
From: Nick Krause @ 2014-09-09 15:52 UTC (permalink / raw)
To: kernelnewbies
On Tue, Sep 9, 2014 at 9:54 AM, Peter Senna Tschudin
<peter.senna@gmail.com> wrote:
> On Tue, Sep 9, 2014 at 3:45 PM, Greg Freemyer <greg.freemyer@gmail.com> wrote:
>>
>>
>> On September 8, 2014 11:08:46 PM EDT, nick <xerofoify@gmail.com> wrote:
>>>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...
>
>
>>
>> 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
>
>
>
> --
> Peter
I understand that , sorry Guys. I am going to fix this later including
my spell checks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 15:52 ` Nick Krause
@ 2014-09-09 16:40 ` Valdis.Kletnieks at vt.edu
2014-09-09 21:16 ` nick
0 siblings, 1 reply; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-09 16:40 UTC (permalink / raw)
To: kernelnewbies
On Tue, 09 Sep 2014 11:52:20 -0400, Nick Krause said:
> I understand that , sorry Guys. I am going to fix this later including
> my spell checks.
Don't bother.
-------------- 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/ee74f91d/attachment.bin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 16:40 ` Valdis.Kletnieks at vt.edu
@ 2014-09-09 21:16 ` nick
2014-09-09 21:33 ` Hugo Mills
2014-09-09 22:03 ` Valdis.Kletnieks at vt.edu
0 siblings, 2 replies; 24+ messages in thread
From: nick @ 2014-09-09 21:16 UTC (permalink / raw)
To: kernelnewbies
On 14-09-09 12:40 PM, Valdis.Kletnieks at vt.edu wrote:
> On Tue, 09 Sep 2014 11:52:20 -0400, Nick Krause said:
>
>> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 21:16 ` nick
@ 2014-09-09 21:33 ` Hugo Mills
2014-09-09 22:03 ` Valdis.Kletnieks at vt.edu
1 sibling, 0 replies; 24+ messages in thread
From: Hugo Mills @ 2014-09-09 21:33 UTC (permalink / raw)
To: kernelnewbies
On Tue, Sep 09, 2014 at 05:16:19PM -0400, nick wrote:
>
>
> On 14-09-09 12:40 PM, Valdis.Kletnieks at vt.edu wrote:
> > On Tue, 09 Sep 2014 11:52:20 -0400, Nick Krause said:
> >
> >> 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.
> Nick
> >From 1d6378589ab97cc646e2a3717413077453e4e80b Mon Sep 17 00:00:00 2001
> From: Nicholas Krause <xerofoify@gmail.com>
> 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 <xerofoify@gmail.com>
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..6988e1c 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -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)
--
=== 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 21:16 ` nick
2014-09-09 21:33 ` Hugo Mills
@ 2014-09-09 22:03 ` Valdis.Kletnieks at vt.edu
2014-09-10 1:21 ` nick
1 sibling, 1 reply; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-09 22:03 UTC (permalink / raw)
To: kernelnewbies
On Tue, 09 Sep 2014 17:16:19 -0400, nick said:
> 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?
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-09 22:03 ` Valdis.Kletnieks at vt.edu
@ 2014-09-10 1:21 ` nick
2014-09-10 2:52 ` Valdis.Kletnieks at vt.edu
0 siblings, 1 reply; 24+ messages in thread
From: nick @ 2014-09-10 1:21 UTC (permalink / raw)
To: kernelnewbies
On 14-09-09 06:03 PM, Valdis.Kletnieks at vt.edu wrote:
> On Tue, 09 Sep 2014 17:16:19 -0400, nick said:
>
>> 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?
>
>> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 1:21 ` nick
@ 2014-09-10 2:52 ` Valdis.Kletnieks at vt.edu
2014-09-10 2:56 ` nick
0 siblings, 1 reply; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-10 2:52 UTC (permalink / raw)
To: kernelnewbies
On Tue, 09 Sep 2014 21:21:18 -0400, nick said:
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 2:52 ` Valdis.Kletnieks at vt.edu
@ 2014-09-10 2:56 ` nick
2014-09-10 3:07 ` Valdis.Kletnieks at vt.edu
0 siblings, 1 reply; 24+ messages in thread
From: nick @ 2014-09-10 2:56 UTC (permalink / raw)
To: kernelnewbies
On 14-09-09 10:52 PM, Valdis.Kletnieks at vt.edu wrote:
> On Tue, 09 Sep 2014 21:21:18 -0400, nick said:
>> 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.
>
Care to explain.
Nick
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 2:56 ` nick
@ 2014-09-10 3:07 ` Valdis.Kletnieks at vt.edu
2014-09-10 3:11 ` nick
0 siblings, 1 reply; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-10 3:07 UTC (permalink / raw)
To: kernelnewbies
On Tue, 09 Sep 2014 22:56:10 -0400, nick said:
> > 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 3:07 ` Valdis.Kletnieks at vt.edu
@ 2014-09-10 3:11 ` nick
2014-09-10 3:53 ` nick
0 siblings, 1 reply; 24+ messages in thread
From: nick @ 2014-09-10 3:11 UTC (permalink / raw)
To: kernelnewbies
On 14-09-09 11:07 PM, Valdis.Kletnieks at vt.edu wrote:
> On Tue, 09 Sep 2014 22:56:10 -0400, nick said:
>
>>> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 3:11 ` nick
@ 2014-09-10 3:53 ` nick
2014-09-10 4:49 ` Valdis.Kletnieks at vt.edu
0 siblings, 1 reply; 24+ messages in thread
From: nick @ 2014-09-10 3:53 UTC (permalink / raw)
To: kernelnewbies
On 14-09-09 11:11 PM, nick wrote:
>
>
> On 14-09-09 11:07 PM, Valdis.Kletnieks at vt.edu wrote:
>> On Tue, 09 Sep 2014 22:56:10 -0400, nick said:
>>
>>>> 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
>
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 3:53 ` nick
@ 2014-09-10 4:49 ` Valdis.Kletnieks at vt.edu
2014-09-10 11:26 ` nick
0 siblings, 1 reply; 24+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-09-10 4:49 UTC (permalink / raw)
To: kernelnewbies
On Tue, 09 Sep 2014 23:53:50 -0400, nick said:
> Build Error. Fixed it. I need to really check my patches first :(.
How many times have you done that now?
> 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.
> + if (skb == NULL) {
> + skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
> + break;
> + }
Like this one - it's *still* wrong. Because....
From: Nicholas Krause <xerofoify@gmail.com>
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 4:49 ` Valdis.Kletnieks at vt.edu
@ 2014-09-10 11:26 ` nick
2014-09-10 11:36 ` Hugo Mills
2014-09-10 11:38 ` Kristofer Hallin
0 siblings, 2 replies; 24+ messages in thread
From: nick @ 2014-09-10 11:26 UTC (permalink / raw)
To: kernelnewbies
On 14-09-10 12:49 AM, Valdis.Kletnieks at vt.edu wrote:
> On Tue, 09 Sep 2014 23:53:50 -0400, nick said:
>> Build Error. Fixed it. I need to really check my patches first :(.
>
> How many times have you done that now?
>
>> 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.
>
>> + if (skb == NULL) {
>> + skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
>> + break;
>> + }
>
> Like this one - it's *still* wrong. Because....
>
> From: Nicholas Krause <xerofoify@gmail.com>
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 11:26 ` nick
@ 2014-09-10 11:36 ` Hugo Mills
2014-09-10 11:38 ` Kristofer Hallin
1 sibling, 0 replies; 24+ messages in thread
From: Hugo Mills @ 2014-09-10 11:36 UTC (permalink / raw)
To: kernelnewbies
On Wed, Sep 10, 2014 at 07:26:32AM -0400, nick wrote:
> On 14-09-10 12:49 AM, Valdis.Kletnieks at vt.edu wrote:
[snip]
> > Like this one - it's *still* wrong. Because....
> >
> > From: Nicholas Krause <xerofoify@gmail.com>
> > Date: Tue, 9 Sep 2014 21:12:56 -0400
> >
> > You reposted the same broken patch again.
[snip]
> Here is the correct patch.
> Nick
> >From ad55b627241203a380dd9c708a4feae98446f85f Mon Sep 17 00:00:00 2001
> From: Nicholas Krause <xerofoify@gmail.com>
> 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 <xerofoify@gmail.com>
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..a251414 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -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.
--
=== 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Trial Patch
2014-09-10 11:26 ` nick
2014-09-10 11:36 ` Hugo Mills
@ 2014-09-10 11:38 ` Kristofer Hallin
1 sibling, 0 replies; 24+ messages in thread
From: Kristofer Hallin @ 2014-09-10 11:38 UTC (permalink / raw)
To: kernelnewbies
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?
On Wed, Sep 10, 2014 at 1:26 PM, nick <xerofoify@gmail.com> wrote:
>
>
> On 14-09-10 12:49 AM, Valdis.Kletnieks at vt.edu wrote:
>> On Tue, 09 Sep 2014 23:53:50 -0400, nick said:
>>> Build Error. Fixed it. I need to really check my patches first :(.
>>
>> How many times have you done that now?
>>
>>> 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.
>>
>>> + if (skb == NULL) {
>>> + skb_queue_purge(&priv->rtllib->skb_waitQ[TXCMD_QUEUE]);
>>> + break;
>>> + }
>>
>> Like this one - it's *still* wrong. Because....
>>
>> From: Nicholas Krause <xerofoify@gmail.com>
>> 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
>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-09-10 11:38 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 3:08 Trial Patch nick
2014-09-09 4:04 ` Valdis.Kletnieks at vt.edu
2014-09-09 12:22 ` nick
2014-09-09 12:42 ` Sudip Mukherjee
2014-09-09 13:24 ` nick
2014-09-09 13:39 ` Sudip Mukherjee
2014-09-09 13:26 ` Valdis.Kletnieks at vt.edu
2014-09-09 13:45 ` Greg Freemyer
2014-09-09 13:54 ` Peter Senna Tschudin
2014-09-09 15:52 ` Nick Krause
2014-09-09 16:40 ` Valdis.Kletnieks at vt.edu
2014-09-09 21:16 ` nick
2014-09-09 21:33 ` Hugo Mills
2014-09-09 22:03 ` Valdis.Kletnieks at vt.edu
2014-09-10 1:21 ` nick
2014-09-10 2:52 ` Valdis.Kletnieks at vt.edu
2014-09-10 2:56 ` nick
2014-09-10 3:07 ` Valdis.Kletnieks at vt.edu
2014-09-10 3:11 ` nick
2014-09-10 3:53 ` nick
2014-09-10 4:49 ` Valdis.Kletnieks at vt.edu
2014-09-10 11:26 ` nick
2014-09-10 11:36 ` Hugo Mills
2014-09-10 11:38 ` Kristofer Hallin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.