From: Pavel Machek <pavel@ucw.cz>
To: Chihau Chau <chihau@gmail.com>
Cc: gregkh@suse.de, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Staging: dream: pmem: fix some code style issues
Date: Tue, 2 Mar 2010 06:22:44 +0100 [thread overview]
Message-ID: <20100302052244.GC13798@elf.ucw.cz> (raw)
In-Reply-To: <1267487415-22061-1-git-send-email-chihau@gmail.com>
On Mon 2010-03-01 20:50:15, Chihau Chau wrote:
> From: Chihau Chau <chihau@gmail.com>
>
> This fixes some code style issues like some braces {} deleted becouse
> are not necessary for a single statement blocks and to include KERN_
> facility level in the printk() functions.
Most of patch is good, but...
> @@ -936,8 +934,8 @@ int pmem_remap(struct pmem_region *region, struct file *file,
> if (unlikely(!PMEM_IS_PAGE_ALIGNED(region->offset) ||
> !PMEM_IS_PAGE_ALIGNED(region->len))) {
> #if PMEM_DEBUG
> - printk("pmem: request for unaligned pmem suballocation "
> - "%lx %lx\n", region->offset, region->len);
> + printk(KERN_ERR "pmem: request for unaligned pmem "
> + "suballocation %lx %lx\n", region->offset, region->len);
> #endif
> return -EINVAL;
> }
This is strange. If it is debuging print, should it be KERN_DEBUG? And
we have nice dev_dbg macros for just that, so that ifdef is not
neccessarry.
> @@ -1087,8 +1085,10 @@ static long pmem_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> region.offset = pmem_start_addr(id, data);
> region.len = pmem_len(id, data);
> }
> - printk(KERN_INFO "pmem: request for physical address of pmem region "
> - "from process %d.\n", current->pid);
> + printk(KERN_INFO "pmem: request for physical address "
> + "of pmem region from process %d.\n",
> + current->pid);
> +
And this gets code worse, not better.
(Feel free to send all the other hunks with my ACK.)
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
next prev parent reply other threads:[~2010-03-02 5:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-01 23:50 [PATCH] Staging: dream: pmem: fix some code style issues Chihau Chau
2010-03-02 5:22 ` Pavel Machek [this message]
2010-04-08 19:07 ` staging: " Greg KH
-- strict thread matches above, loose matches on Subject: below --
2010-04-10 19:14 [PATCH] Staging: " Chihau Chau
2010-04-10 20:47 ` Pavel Machek
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=20100302052244.GC13798@elf.ucw.cz \
--to=pavel@ucw.cz \
--cc=chihau@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@suse.de \
--cc=linux-kernel@vger.kernel.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 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.