[GiNaC-devel] Signed versus unsigned (Was: Wrong patch)

[Alexei Sheplyakov]

Hello,

On Wed, Apr 06, 2011 at 09:33:50AM +0200, Richard B. Kreckel wrote:

> Alexei, does it really break the ABI? Whether we return an int here
> or a char won't change library symbol names and since chars are
> returned as ints, it won't change anything binary-wise.

Changing the return type might break the ABI, and I think it's better to
be safe than sorry.

> There are cases where clifford_max_label returns -1, so this is
> indeed broken.

clifford_max_label returns some magic number (-1 aka 255) to indicate that
the expression does not contain any gamma matrices. It's perfectly fine to
do so as long as no valid representation label coincides with that magic
number. 

Let's forget about computer algebra for a second and consider the following
(C) code:

#include <stdlib.h> /* size_t */

int max_val_s(const unsigned int* data, size_t sz)
{
	size_t i = 0;
	unsigned int val = 0;
	if ((!data) || (!sz))
		return -1; 
		/* -1 is just a magic number which means "the input is empty */

	for (; i < sz; ++i) {
		if (val < data[i])
			val = data[i];
	}
	return val;
}


The max_val_s is perfectly fine as long as the input data does not contain
values greater than INT_MAX (usually 2^31 - 1).

unsigned int max_val_u(const unsigned int* data, size_t sz)
{
	size_t i = 0;
	unsigned int val = 0;

	if ((!data) || (!sz))
		return -1;
		/* -1 is just a magic number which means "the input is empty */

	for (; i < sz; ++i) {
		if (val < data[i])
			val = data[i];
	}
	return val;
}

The max_val_u is fine, too, as long as the input values are less than UINT_MAX.

The above statements still hold if one replaces int with char (INT_MAX with
CHAR_MAX, and UINT_MAX with UCHAR_MAX, respectively). Thus, clifford_max_label
is OK as long as representation labels are within [0, 127] range (no matter if
char is signed or not).

> So, I don't see a problem with Martin's patch. Did I miss anything?

Apart from a possible ABI change the patch is harmless. On the other hand,
it's useless (see the explanation above), so I don't see any reason to
apply it (as in "if it ain't broke, don't fix it").

Best regards,
	Alexei