lkml.org 
[lkml]   [2008]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Fix cpuset sched_relax_domain_level control file
On Wed, 7 May 2008 12:48:09 +0300 Adrian Bunk <bunk@kernel.org> wrote:

> On Tue, May 06, 2008 at 06:41:39PM -0700, Paul Menage wrote:
> > On Tue, May 6, 2008 at 6:38 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Tue, 06 May 2008 18:08:17 -0700 Paul Menage <menage@google.com> wrote:
> > >
> > > > -static int update_relax_domain_level(struct cpuset *cs, char *buf)
> > > > +static int update_relax_domain_level(struct cpuset *cs, s64 val)
> > > > {
> > > > - int val = simple_strtol(buf, NULL, 10);
> > > > -
> > > > - if (val < 0)
> > > > + if ((int)val < 0)
> > > > val = -1;
> > > >
> > >
> > > Are you sure about the typecast here? If `val' has a value of say
> > > 0x0000_ffff_ffff_ffff then I assume the casted value will be negative, only
> > > it wasn't?
> >
> > It seems like the simplest approach - if it's outside the range of a
> > positive int, set it to -1.
>
> That's very hard to understand for someone who looks at the code - and
> being able to understand the code is much more important than the
> number of characters in the source code.
>
> If you'd write something like
>
> if ((val < 0) || (val > INT_MAX))
>
> instead it would be obvious for the reader what's happening here, and
> that this was intended.

What he said.


Our poor reader now knows what was intended. But he still doesn't know _why_
it was intended.



\
 
 \ /
  Last update: 2008-05-07 17:11    [W:0.083 / U:0.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site