From: Dave Hansen <haveblue@us.ibm.com>
To: Ryan Arnold <rsa@us.ibm.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
greg@kroah.com, boutcher@us.ibm.com, rsa@us.ibm.com,
Hollis Blanchard <hollisb@us.ibm.com>
Subject: Re: new driver (hvcs) review request and sysfs questions
Date: Wed, 25 Feb 2004 08:22:33 -0800 [thread overview]
Message-ID: <1077726152.22018.403.camel@nighthawk> (raw)
In-Reply-To: <1077667227.21201.73.camel@SigurRos.rchland.ibm.com>
--- pristine/linux-2.5/drivers/char/hvcs.c ...
+++ working/linux-2.5/drivers/char/hvcs.c ...
It's standard practice to create patches so that they apply with -p1.
See Documentation/SubmittingPatches
Don't just indiscriminately run lindent. It makes the code look awful
in some spots. Remember that it's OK to exceed 80 columns in some
places, especially with things like function declarations.
in hvcs_write():
count = min(count, (int)PAGE_SIZE);
assumes that count and PAGE_SIZE are in the same units: bytes. If
that's true, no need for the sizeof() here:
charbuf = kmalloc(sizeof(unsigned char *) * count, GFP_KERNEL);
hvcs_next_partner() shares code with lparcfg_write() and a bunch of the
other hypervisor calls. Can you combine them with a function that just
checks the return codes from hcalls? All of those switches look
redundant.
hvcs_get_partner_info() is a bit long. Can that while loop go into its
own function?
-- dave
prev parent reply other threads:[~2004-02-25 16:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-02-25 0:00 new driver (hvcs) review request and sysfs questions Ryan Arnold
2004-02-25 1:28 ` Greg KH
2004-02-25 3:12 ` Dave Boutcher
2004-02-25 4:22 ` Greg KH
2004-02-25 11:34 ` Benjamin Herrenschmidt
2004-02-25 15:46 ` device/kobject naming Hollis Blanchard
2004-02-26 0:18 ` Greg KH
2004-02-26 21:31 ` Hollis Blanchard
2004-02-25 16:22 ` Dave Hansen [this message]
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=1077726152.22018.403.camel@nighthawk \
--to=haveblue@us.ibm.com \
--cc=boutcher@us.ibm.com \
--cc=greg@kroah.com \
--cc=hollisb@us.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rsa@us.ibm.com \
/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