2013-05-17 14:38:41

by Joakim Tjernlund

Subject: Is the code stateid_generation_after()in legal C?

static bool stateid_generation_after(stateid_t *a, stateid_t *b)
{
return (s32)a->si_generation - (s32)b->si_generation > 0;
}

overflow is undefined for signed integers and gcc uses that nowadays.
Not sure if that can affect the above code?

Jocke

2013-05-18 12:10:12

by Joakim Tjernlund

Subject: Re: Is the code stateid_generation_after()in legal C?

Jim Rees <[email protected]> wrote on 2013/05/17 18:50:38:
>
> Joakim Tjernlund wrote:
>
> static bool stateid_generation_after(stateid_t *a, stateid_t *b)
> {
> return (s32)a->si_generation - (s32)b->si_generation > 0;
> }
>
> overflow is undefined for signed integers and gcc uses that nowadays.
> Not sure if that can affect the above code?
>
> I guess the intent there is to account for stateid wraparound. But it's
not
> clear to me this is doing the right thing. I think C specifies overflow
> behavior for unsigned but not signed.

Right, the below test prog demonstrates the signed overflow problem in
newer gcc's
#include <assert.h>

int foo(int a) {
assert(a+100 > a);
printf("%d %d\n",a+100,a);
return a;
}

int main() {
foo(100);
foo(0x7fffffff);
}

> So shouldn't it be something more like this?
>
> (s32)(a->si_generation - b->si_generation) > 0
>
> Either way, this probably deserves a comment.

2013-05-17 16:50:46

by Jim Rees

Subject: Re: Is the code stateid_generation_after()in legal C?

Joakim Tjernlund wrote:

static bool stateid_generation_after(stateid_t *a, stateid_t *b)
{
return (s32)a->si_generation - (s32)b->si_generation > 0;
}

overflow is undefined for signed integers and gcc uses that nowadays.
Not sure if that can affect the above code?

I guess the intent there is to account for stateid wraparound. But it's not
clear to me this is doing the right thing. I think C specifies overflow
behavior for unsigned but not signed. So shouldn't it be something more like
this?

(s32)(a->si_generation - b->si_generation) > 0

Either way, this probably deserves a comment.