[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <200809121213.45877.david-b@pacbell.net>
Date: Fri, 12 Sep 2008 12:13:45 -0700
From: David Brownell <david-b@...bell.net>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: rtc-linux@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [patch 2.6.27 mmotm] rtc-cmos: export second NVRAM bank
On Friday 12 September 2008, Andrew Morton wrote:
> > +#define can_bank2 true
>
> It would be more idiomatic to make this upper-case: CAN_BANK2.
Think of it as: "static const bool can_bank2 = true;" ...
> : static ssize_t
> : cmos_nvram_read(struct kobject *kobj, struct bin_attribute *attr,
> : char *buf, loff_t off, size_t count)
> : {
> : int retval;
> :
> : if (unlikely(off >= attr->size))
> : return 0;
> : if (unlikely(off < 0))
> : return -EINVAL;
> : if ((off + count) > attr->size)
> : count = attr->size - off;
> :
> : off += NVRAM_OFFSET;
> :
>
> The VFS will (hopefully) prevent ->read methods from being called with
> a negative file offset. What prompted the additional test for that?
I was chasing down a problem related to offsets. Now fixed;
that bit could probably go away ... except that so long as the
interface allows negative/bogus values, it seems appropriate to
defend against them. If the interface passed an unsigned type,
my paranoia-meter would not fire in this case.
> I did't look at it exhaustively but I suspect that the above code won't
> work right if attr->size has a value of around (2^31 - 42) and `offset'
> is (2^31 - 54) and NVRAM_OFFSET==54. Or something like that. It looks
> holey ;)
In this case, size over 256 would be impossible. A relevant
case study may be:
http://thedailywtf.com/Articles/Rule-Number-One.aspx
:)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists