Re: {put,get}_user() side effects

Re: {put,get}_user() side effects

[Andi Kleen]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> On most (all?) architectures {get,put}_user() has side effects:
>
> #define put_user(x,ptr)                                                 \
>   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))

Neither typeof not sizeof are supposed to have side effects. If your
compiler generates them that's a compiler bug.

-Andi

Re: {put,get}_user() side effects

[Geert Uytterhoeven]

On Tue, 6 Apr 2004, Andi Kleen wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> > On most (all?) architectures {get,put}_user() has side effects:
> >
> > #define put_user(x,ptr)                                                 \
> >   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
>
> Neither typeof not sizeof are supposed to have side effects. If your
> compiler generates them that's a compiler bug.

>From a simple compile test, you seem to be right... Weird, since it does expand
to 3 times 'pIndex++', but pIndex is incremented only once.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: {put,get}_user() side effects

[Horst von Brand]

Geert Uytterhoeven <geert@linux-m68k.org> said:
> On Tue, 6 Apr 2004, Andi Kleen wrote:
> > Geert Uytterhoeven <geert@linux-m68k.org> writes:
> > > On most (all?) architectures {get,put}_user() has side effects:
> > >
> > > #define put_user(x,ptr)                                                 \
> > >   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
> >
> > Neither typeof not sizeof are supposed to have side effects. If your
> > compiler generates them that's a compiler bug.

> From a simple compile test, you seem to be right... Weird, since it does
> expand to 3 times 'pIndex++', but pIndex is incremented only once.

Better check with a C language lawyer. Maybe gcc gets it wrong, or it is
undefined (in which case next gcc could screw you over, and give you a hard
time finding out how...). An inline should be safe, and unless gcc still
gets them wrong, equally fast/small.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: {put,get}_user() side effects

[Andreas Schwab]

Horst von Brand <vonbrand@inf.utfsm.cl> writes:

> Geert Uytterhoeven <geert@linux-m68k.org> said:
>> On Tue, 6 Apr 2004, Andi Kleen wrote:
>> > Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> > > On most (all?) architectures {get,put}_user() has side effects:
>> > >
>> > > #define put_user(x,ptr)                                                 \
>> > >   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
>> >
>> > Neither typeof not sizeof are supposed to have side effects. If your
>> > compiler generates them that's a compiler bug.
>
>> From a simple compile test, you seem to be right... Weird, since it does
>> expand to 3 times 'pIndex++', but pIndex is incremented only once.
>
> Better check with a C language lawyer. Maybe gcc gets it wrong, or it is
> undefined

It's not undefined, the standard explicitly says that the argument of
sizeof is not evaluated (unless its type is a VLA).  I can't remember gcc
ever getting that wrong.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: {put,get}_user() side effects

[Xavier Bestel]

On Tue, 2004-04-06 at 13:32 +0200, Andi Kleen wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 
> > On most (all?) architectures {get,put}_user() has side effects:
> >
> > #define put_user(x,ptr)                                                 \
> >   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
> 
> Neither typeof not sizeof are supposed to have side effects. If your
> compiler generates them that's a compiler bug.

Using ptr three times in a define has side effects if ptr is an
expression with side effects (e.g. "p++").

	Xav

Re: {put,get}_user() side effects

[Wichert Akkerman]

Previously Xavier Bestel wrote:
> Using ptr three times in a define has side effects if ptr is an
> expression with side effects (e.g. "p++").

As I understand it both typeof and sizeof don't evalutate their argument
but only look at its type. Which means using p++ is perfectly safe.

Wichert.

-- 
Wichert Akkerman <wichert@wiggy.net>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.

Re: {put,get}_user() side effects

[Xavier Bestel]

> On Tue, 2004-04-06 at 13:32 +0200, Andi Kleen wrote:
> > Geert Uytterhoeven <geert@linux-m68k.org> writes:
> > 
> > > On most (all?) architectures {get,put}_user() has side effects:
> > >
> > > #define put_user(x,ptr)                                                 \
> > >   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))
> > 
> > Neither typeof not sizeof are supposed to have side effects. If your
> > compiler generates them that's a compiler bug.
> 
> Using ptr three times in a define has side effects if ptr is an
> expression with side effects (e.g. "p++").

Sorry, I read too fast. I didn't know sizeof could avoid side effects.

	Xav

Re: {put,get}_user() side effects

[Kevin P. Fleming]

Xavier Bestel wrote:

> Sorry, I read too fast. I didn't know sizeof could avoid side effects.
> 

Both typeof and sizeof are compile-time constructs, so there is no 
opportunity for any expression side effects to occur. Presumably one 
could do:

typeof(1/0UL)

without ever causing the obvious side effect either (granted, this is a 
pointless piece of code :-)).

Re: {put,get}_user() side effects

[Xavier Bestel]

On Tue, 2004-04-06 at 08:30 -0700, Kevin P. Fleming wrote:
> Xavier Bestel wrote:
> 
> > Sorry, I read too fast. I didn't know sizeof could avoid side effects.
> > 
> 
> Both typeof and sizeof are compile-time constructs, so there is no 
> opportunity for any expression side effects to occur. Presumably one 
> could do:
> 
> typeof(1/0UL)
> 
> without ever causing the obvious side effect either (granted, this is a 
> pointless piece of code :-)).

Yeah, now that you tell it, it seems obvious when you consider macros
like min/max and friends which rely on typeof to avoid side-effects.

{put,get}_user() side effects

[Geert Uytterhoeven]

On most (all?) architectures {get,put}_user() has side effects:

#define put_user(x,ptr)                                                 \
  __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))


E.g. drivers/char/ip2main.c seems to be completely broken due to constructs
like:

    rc = put_user(i2Input, pIndex++ );

I only noticed these because the compiler gave some warnings due to other
reasons.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: {put,get}_user() side effects

[Russell King]

On Tue, Apr 06, 2004 at 12:03:14PM +0200, Geert Uytterhoeven wrote:
> On most (all?) architectures {get,put}_user() has side effects:
> 
> #define put_user(x,ptr)                                                 \
>   __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr)))

I thought this came up before, and it was decided that put_user and
get_user must not have such side effects - I certainly remember fixing
this very thing on ARM.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core