[GiNaC-devel] Wrong patch (Was: Fix unsigned character bugs in ginac-1.5.8)

[Martin Guy]

On 2 April 2011 14:06, Alexei Sheplyakov <alexei.sheplyakov at gmail.com> wrote:
> On Sat, Apr 02, 2011 at 12:08:24PM +0200, Martin Guy wrote:
>> This patch fixes a bug on machines where char is unsigned by default, by
>> extending the type of clifford_max_label to include all 257 possiblet
>> return values.
>
> I'm sorry to confuse you, but your patch is wrong. It just breaks ABI for
> nothing good at all. In practice there's no need for gamma matrices with
> 128 representation labels (otherwise GiNaC would have problems on machines
> where char is *signed* by default). I guess we should fix the documentation
> (it claims that representation labels from 0 to 255 are supported, which
> is definitely wrong).

Eh? On unsigned char systems it currently fails to detect an error
condition, but continues blindly,
wihle users who have spotted the defect are forced to detect errors
using "if (x & 255 == EOF & 255), which is crazy.

In practice, chars are passed and returned as ints, so nothing changes
in the function call/return values (which is what I thought an ABI
was).

Since users presumably include clifford.h before calling the function,
at most they will get a compiler warning about an integer being stored
in a character if they have done that, which also correctly pinpoints
the error in their code.

Anyway

    M