From: Andrew Morton <akpm@linux-foundation.org>
To: David Vrabel <david.vrabel@csr.com>
Cc: torvalds@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] UWB, WUSB, and WLP subsystems for 2.6.28
Date: Tue, 14 Oct 2008 13:02:51 -0700 [thread overview]
Message-ID: <20081014130251.aa008a5e.akpm@linux-foundation.org> (raw)
In-Reply-To: <48EF45BC.1020805@csr.com>
> On Fri, 10 Oct 2008 13:08:28 +0100 David Vrabel <david.vrabel@csr.com> wrote:
> Please pull the new UWB, WUSB and WLP subsystems from
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dvrabel/uwb.git for-upstream
didn't happen?
What is the review status of this work? I don't remember seeing it on any
of the lists where I lurk - perhaps a full resend will help things along.
<quick scan>
Code looks reasonable.
It has lots of comments which start with /**, which is the
this-is-kerneldoc token. Only they're not kerneldoc comments. These
should all be converted to kerneldoc, or replace the /** with /*.
uwb_beca_purge() should use time_after() or time_before().
In uwb_bce_print_IEs(), the cast of
uwb_rc_evt_beacon_WUSB_0100.BeaconInfo[] into a struct uwb_rc_evt_beacon*
looks really worrisome from an alignment POV. Can it result in misaligned
accesses on architectures which don't like that? (ia64, alpha, ...)
Code does kzalloc(a * b, ..) in some places. kcalloc() is preferred, so
readers don't have to worry whether the code is vulnerable to
multiplicative overflows.
The code has a random mixture of
zero-lines-between-end-of-locals-and-start-of-code and
one-line-between-end-of-locals-and-start-of-code (and two line). The
latter is usually preferred.
The person who misnamed DEFINE_BITMAP as DECLARE_BITMAP instead gets a
wedgie.
It seems strange that uwb_drp_ie_update(UWB_RSV_STATE_NONE) will free
rsv->drp_ie then reallocate it.
printk_ratelimit() is a bit silly because it shares state with other
unrelated subsystems which might be using it. Direct use of __ratelimit()
would be better.
All minor stuff - I didn't spend long looking...
next prev parent reply other threads:[~2008-10-14 20:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-10 12:08 [GIT PULL] UWB, WUSB, and WLP subsystems for 2.6.28 David Vrabel
2008-10-14 20:02 ` Andrew Morton [this message]
2008-10-15 12:50 ` David Vrabel
2008-10-16 13:20 ` David Vrabel
2008-10-21 16:44 ` Marcel Holtmann
2008-10-17 16:50 ` Marcel Holtmann
-- strict thread matches above, loose matches on Subject: below --
2008-10-22 14:20 David Vrabel
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=20081014130251.aa008a5e.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=david.vrabel@csr.com \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.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.