[Pixman] warnings in pixman 0.21.4

[Siarhei Siamashka]

On Monday 24 January 2011 11:51:39 Rolland Dudemaine wrote:
> - *max_vx.diff corrects the initialization of max_vx. Contrary to what
> you mention, the variable is declared in a local block, so the scope of
> the variable is restricted to that block, regardless of whether it is
> part of a macro or not. So the compiler is correct saying the value may
> be used before being initialized.

Used where?

> Also, INT32_MAX is a safe value,

Yes, just like any other value. And INT32_MAX is misleading because it
may make somebody think that there is some meaning behind it. Additionally
it makes an assumption that pixman_fixed_t is a 32-bit integer type, which
may be not totally future proof (admittedly there are other parts of pixman
making this assumption too).

> because pixman-compiler.h defines it. INT32_MAX is the right value to
> use since max_vx is a clamping value passed on to a callee. 0 would just
> clamp down to 0, which would be plainly incorrect.

When the code is expanded and optimized to remove parts which depend on 
constant expressions which evaluate to false, it looks somewhat similar
to this:

int __attribute__((always_inline)) f1(int a0, int a1, int a2)
{
    return a0 + a1;
}

int f2(int *a)
{
    int a2;
    return f1(a[0], a[1], a2);
}

Trying to pass undefined variable as an argument to a function is wrong just
because we want to have correct and fully standard conformant C code, so I 
agree that this needs to be corrected. But we really don't care about the value
of this variable because it is not really used in the sense of affecting
program logic.


Actually what you propose is not totally meaningless. And we might be able to
get rid of the NORMAL repeat check here and improve code readability a bit if
'max_vx' variable was changed from being just beyond the valid range to
become the biggest valid 'vx' value inclusive.

    vx += unit_x;
    if (PIXMAN_REPEAT_ ## repeat_mode == PIXMAN_REPEAT_NORMAL)
    {
        /* This works because we know that unit_x is positive */
        while (vx >= max_vx)
            vx -= max_vx;
    }

In this case the compiler should be able to eliminate 'while (vx > max_vx)'
loop when 'max_vx' is set to the maximum possible value and is known at
compile time. And hopefully none of the compilers would be annoying enough
to spit some warnings about this :)

Still, having to use 'vx -= max_vx + 1' in this loop would slowdown NORMAL
repeat, so I don't see much point in attempting this change.

-- 
Best regards,
Siarhei Siamashka