linux-kernel - Re: [PATCH] fix round

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20090701130218.29887c54.akpm@linux-foundation.org>
Date:	Wed, 1 Jul 2009 13:02:18 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Joe Perches <joe@...ches.com>
Cc:	yinghai@...nel.org, hpa@...or.com, mingo@...e.hu,
	tglx@...utronix.de, torvalds@...ux-foundation.org, mikpe@...uu.se,
	matthew@....cx, grundler@...isc-linux.org,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH] fix round_up/down

On Wed, 01 Jul 2009 12:39:35 -0700
Joe Perches <joe@...ches.com> wrote:

> On Wed, 2009-07-01 at 12:33 -0700, Yinghai Lu wrote:
> > --- linux-2.6.orig/include/linux/kernel.h
> > +++ linux-2.6/include/linux/kernel.h
> []
> >  #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
> > +#define rounddown(x, y) (((x) / (y)) * (y))
> []
> > +#define round_up(x,y) ((((x)-1) | __round_mask(x,y))+1)
> > +#define round_down(x,y) ((x) & ~__round_mask(x,y))
> 
> Isn't this just asking for trouble?

Yes, I think so.  round_up() versus roundup() is a bit subtle!

> How about a better name?
> Maybe masked_roundup, or roundup_ala_zorro...

Yes.  roundup() wasn't a well-chosen identifier, really.  But I guess
it's compatible with the faster bitwise-based rounding operation so
it's OK to have a special fast version of roundup() for the cases we're
rounding up to a power-of-2.

umm, how about roundup_pow2()?  Sucks?

Also, it it lower-case or all-caps?  I think it should be all-caps.  Because

a) it is a macro, and it can ONLY be implemented as a macro, so
   there's no point in pretending that it might be a C function and that the
   caller needn't care.

b) several of these macros evaluate their args multiple times and
   hence will produce buggy or inefficeint code when passed expressions
   with side-effects.  So we should warn people that these things are macros.

   We should also fix that, dammit.  The proposed new rounddown()
   above has this failing as well.

--
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/