From: Valdis.Kletnieks@vt.edu (Valdis.Kletnieks at vt.edu)
To: kernelnewbies@lists.kernelnewbies.org
Subject: Trial Patch
Date: Tue, 09 Sep 2014 00:04:06 -0400 [thread overview]
Message-ID: <102131.1410235446@turing-police.cc.vt.edu> (raw)
In-Reply-To: Your message of "Mon, 08 Sep 2014 23:08:46 -0400." <540E6F3E.60802@gmail.com>
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
next prev parent reply other threads:[~2014-09-09 4:04 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-09 3:08 Trial Patch nick
2014-09-09 4:04 ` Valdis.Kletnieks at vt.edu [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=102131.1410235446@turing-police.cc.vt.edu \
--to=valdis.kletnieks@vt.edu \
--cc=kernelnewbies@lists.kernelnewbies.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).