Why is this cast nessasary

[Neil]

Question

should this cast be necessary?
The Compiler is 8 Bits the ints are 16 bits.
The Code is for a ring buffer

It does not work without the cast in this compiler.
I have seen it work else where without it.
Which way is right (or should it matter at all?)

Thanks Neil







unsigned char t_in;
unsigned char t_out;

/*----------------------------------------------------------------*/
bit com_putchar (unsigned char c)
{

/*If the buffer is full, return an error value.*/
if ((TBUF_SIZE - (unsigned char)(t_in - t_out)) <= 2)
return (1);

/* More Code */
/* More Code */
/* More Code */
/* More Code */



return (0);
}

 

[Keith Thompson]

Question

should this cast be necessary?
The Compiler is 8 Bits the ints are 16 bits.

I don't know what you mean by "The Compiler is 8 Bits". I'll assume
char is 8 bits (CHAR_BIT==8) and sizeof(int)==2.

The Code is for a ring buffer

It does not work without the cast in this compiler.

It's impossible to tell what you mean by "does not work". Does it
fail to compile? Does it give you different results than the ones you
expected? What did you expect, and what did you get?

I have seen it work else where without it.
Which way is right (or should it matter at all?)
[...]
..
unsigned char t_in;
unsigned char t_out;

/*----------------------------------------------------------------*/
bit com_putchar (unsigned char c)

Presumably "bit" is declared somewhere.

{

/*If the buffer is full, return an error value.*/
if ((TBUF_SIZE - (unsigned char)(t_in - t_out)) <= 2)

Likewise TBUF_SIZE.

In an expression, any subexpression of type unsigned char is going to
be promoted to either int or unsigned int. It will be promoted to int
if int can hold all possible values of type unsigned char (which is
almost always the case).

So t_in - t_out is of type int, and it could be either positive,
negative, or zero. The conversion to unsigned char yields a positive
or zero result. For example, if t_in - t_out is -1, the conversion
probably yields 255.

You then subtract that result from TBUF_SIZE, but we have no idea of
the value, or even the type, of TBUF_SIZE.

That's about all I can tell you.

return (1);

/* More Code */ [...]

return (0);
}

A minor style point: The parentheses on the return statements are not
necessary. They're harmless, but I (and, I think, most c programers)
prefer:
return 1;
...
return 0;

 

[Neil]

Question

should this cast be necessary?
The Compiler is 8 Bits the ints are 16 bits.

I don't know what you mean by "The Compiler is 8 Bits". I'll assume
char is 8 bits (CHAR_BIT==8) and sizeof(int)==2.

The Code is for a ring buffer

It does not work without the cast in this compiler.

It's impossible to tell what you mean by "does not work". Does it
fail to compile? Does it give you different results than the ones you
expected? What did you expect, and what did you get?

I have seen it work else where without it.
Which way is right (or should it matter at all?)
[...]
.
unsigned char t_in;
unsigned char t_out;

/*----------------------------------------------------------------*/
bit com_putchar (unsigned char c)

Presumably "bit" is declared somewhere.

{

/*If the buffer is full, return an error value.*/
if ((TBUF_SIZE - (unsigned char)(t_in - t_out)) <= 2)

Likewise TBUF_SIZE.

In an expression, any subexpression of type unsigned char is going to
be promoted to either int or unsigned int. It will be promoted to int
if int can hold all possible values of type unsigned char (which is
almost always the case).

So t_in - t_out is of type int, and it could be either positive,
negative, or zero. The conversion to unsigned char yields a positive
or zero result. For example, if t_in - t_out is -1, the conversion
probably yields 255.

You then subtract that result from TBUF_SIZE, but we have no idea of
the value, or even the type, of TBUF_SIZE.

That's about all I can tell you.

return (1);

/* More Code */ [...]
return (0);
}

A minor style point: The parentheses on the return statements are not
necessary. They're harmless, but I (and, I think, most c programers)
prefer:
return 1;
...
return 0;

Sorry I was Not complete:
8 bit as in PIC16 CPU not a PC. 8 bit char and 16 bit ints
bit is actually a bit, but I assume it does not matter.

TBUF_SIZE Assume 32 for this purpose.

The parentheses I try to keep the code uniform. the original must have
had them.

Does not work means compiles, but does not work as expected. The math
does not work as the ring buffer wraps. The full buffer is not detected
and the data is over written.

So it could be an integer promotion issue (that would make sense in this
case). The one other Compiler has a none standard No promote option.
And the second was for a 32 bit compiler using unsigned longs

I hate fix something without understanding the fix, or complain about
the compiler when it doing the right thing. I could look at it again
and see if the compiler matches the math if it is all promoted.

 

[Andrey Tarasevich]

...

You still forgot to mention one important detail: what is the _type_ of 'TBUF_SIZE'?

Let's assume that 'TBUF_SIZE' has a relatively "large" unsigned type. For
example, let's assume that 'TBUF_SIZE' is of type 'size_t'. Let's also assume
that 'TBUF_SIZE's value is equal to 256.

Now, consider what will happen if we remove the cast

if ((TBUF_SIZE - (t_in - t_out)) <= 2)

and try to evaluate this in situation when 't_in' is less than 't_out'. Integral
promotion takes place and assume the 't_in - t_out' is evaluated withing the
bounds of signed type 'int'. Let say we get -1 as the result, for example. Now,
it's time to subtract the result from 'TBUF_SIZE'. Since 'TBUF_SIZE' is of large
unsigned type, the whole expression is evaluated within the bounds of that
unsigned type, meaning that the -1 is converted to that usigned type first. In
this case -1 turns into a large unsigned value. When we subtract that large
unsigned value from 256 we run into the wrap-around behavior and the final
result looks as if we in fact subtracted -1 from 256, i.e. we get 257. The
'if's condition is not satisfied.

Now, consider the same with the cast

if ((TBUF_SIZE - (unsigned char)(t_in - t_out)) <= 2)

The first subtraction still gives us signed valuue -1. But then we explicitly
convert it to 'unsigned char', which (assuming 8-bit chars) gives us 255. The
following subtraction from 'TBUF_SIZE' will give us 1. Now the 'if's condition
is satisfied.

The above desribes two possible scenarios when the behavior of the code with the
cast is different from that of the code without the cast. That's what might happen.

 

[Neil]

You still forgot to mention one important detail: what is the _type_ of 'TBUF_SIZE'?

Let's assume that 'TBUF_SIZE' has a relatively "large" unsigned type. For
example, let's assume that 'TBUF_SIZE' is of type 'size_t'. Let's also assume
that 'TBUF_SIZE's value is equal to 256.

Now, consider what will happen if we remove the cast

if ((TBUF_SIZE - (t_in - t_out)) <= 2)

and try to evaluate this in situation when 't_in' is less than 't_out'. Integral
promotion takes place and assume the 't_in - t_out' is evaluated withing the
bounds of signed type 'int'. Let say we get -1 as the result, for example. Now,
it's time to subtract the result from 'TBUF_SIZE'. Since 'TBUF_SIZE' is of large
unsigned type, the whole expression is evaluated within the bounds of that
unsigned type, meaning that the -1 is converted to that usigned type first. In
this case -1 turns into a large unsigned value. When we subtract that large
unsigned value from 256 we run into the wrap-around behavior and the final
result looks as if we in fact subtracted -1 from 256, i.e. we get 257. The
'if's condition is not satisfied.

Now, consider the same with the cast

if ((TBUF_SIZE - (unsigned char)(t_in - t_out)) <= 2)

The first subtraction still gives us signed valuue -1. But then we explicitly
convert it to 'unsigned char', which (assuming 8-bit chars) gives us 255. The
following subtraction from 'TBUF_SIZE' will give us 1. Now the 'if's condition
is satisfied.

The above desribes two possible scenarios when the behavior of the code with the
cast is different from that of the code without the cast. That's what might happen.

TBUF_SIZE is #define TBUF_SIZE 16

I think it should be #define TBUF_SIZE 16U

I am sure the issue is integer promotion, I did not consider that at all.

I will check the math when I get a chance to single step through it again.

Thanks All Neil.

 

[Thad Smith]

should this cast be necessary?
The Compiler is 8 Bits the ints are 16 bits.
The Code is for a ring buffer

It does not work without the cast in this compiler.

unsigned char t_in;
unsigned char t_out;

/*----------------------------------------------------------------*/
bit com_putchar (unsigned char c)
{

/*If the buffer is full, return an error value.*/
if ((TBUF_SIZE - (unsigned char)(t_in - t_out)) <= 2)
return (1);

Assuming a standard implementation where t_in and t_out are input and
output indexes, the logic is incorrect after a buffer wrap. Assume
buffer size is 10, t_in=9 and t_out=5. Depending on the definition of
the subscripts (current vs. last vs. next character), lets assume they
indicate 4 characters in the buffer, the value of (t_in - t_out). When
one character is added and t_in wraps to 0, (unsigned char)(t_in -
t_out) evaluates to (unsigned char)(-5) = -5 + UCHAR_MAX+1. For 8-bit
characters, that is 251, not the number of characters in the buffer.

The actual number of characters in buffer (with the above assumption) is
t_in > t_out ? (t_in - t_out) : (TBUF_SIZE - 1 - t_out) + t_in;

To get the number of unused locations, subtract from the buffer size.

The result is that the expression was not correct either with or without
the cast, since buffer wrap wasn't handled correctly.

 

[Neil]

Assuming a standard implementation where t_in and t_out are input and
output indexes, the logic is incorrect after a buffer wrap. Assume
buffer size is 10, t_in=9 and t_out=5. Depending on the definition of
the subscripts (current vs. last vs. next character), lets assume they
indicate 4 characters in the buffer, the value of (t_in - t_out). When
one character is added and t_in wraps to 0, (unsigned char)(t_in -
t_out) evaluates to (unsigned char)(-5) = -5 + UCHAR_MAX+1. For 8-bit
characters, that is 251, not the number of characters in the buffer.

The actual number of characters in buffer (with the above assumption) is
t_in > t_out ? (t_in - t_out) : (TBUF_SIZE - 1 - t_out) + t_in;

To get the number of unused locations, subtract from the buffer size.

The result is that the expression was not correct either with or without
the cast, since buffer wrap wasn't handled correctly.

You are right it is still wrong. I hate it when it looks like it works,
but is wrong)
But it can be done continuous, not 2 cases.
The original two examples I used Use the full width of the type.

Assuming my buffer was 256 bytes with 8bit indexes I get a modulus 256
for free.

Since it is smaller I must truncate myself.

if ((TBUF_SIZE - ((t_in - t_out)%TBUF_SIZE) <= 2)

in Excel:

Buff
Size tin tout tin-tout tin-tout%16 size-(tin-tout)
16 13 5 8 8 8
16 14 5 9 9 7
16 15 5 10 10 6
16 0 5 -5 11 5
16 1 5 -4 12 4
16 2 5 -3 13 3
16 3 5 -2 14 2

Does this look any better?
Neil

 

[Thad Smith]

You are right it is still wrong. I hate it when it looks like it works,
but is wrong)
But it can be done continuous, not 2 cases.
The original two examples I used Use the full width of the type.

Assuming my buffer was 256 bytes with 8bit indexes I get a modulus 256
for free.

Since it is smaller I must truncate myself.

if ((TBUF_SIZE - ((t_in - t_out)%TBUF_SIZE) <= 2)

in Excel:

Buff
Size tin tout tin-tout tin-tout%16 size-(tin-tout)
16 13 5 8 8 8
16 14 5 9 9 7
16 15 5 10 10 6
16 0 5 -5 11 5
16 1 5 -4 12 4
16 2 5 -3 13 3
16 3 5 -2 14 2

Does this look any better?

Better, but still not correct in C. The result a%b has the same sign as
a if the results is non-zero. You can get what you want by making it
positive:
(t_in - t_out + TBUF_SIZE) % TBUF_SIZE

 

[CBFalconer]

.... snip ...

Better, but still not correct in C. The result a%b has the same
sign as a if the results is non-zero. You can get what you want
by making it positive:
(t_in - t_out + TBUF_SIZE) % TBUF_SIZE

Caution. There is a subtle difference in the modulo operation on
negative values between C90 and C99. I am not sure exactly what,
but I know it is there. Check the standard(s).

 

[Neil]

Better, but still not correct in C. The result a%b has the same sign as
a if the results is non-zero. You can get what you want by making it
positive:
(t_in - t_out + TBUF_SIZE) % TBUF_SIZE

Thanks
That is actualy what I had in Excel
I am glad I posted
Or, I may have just left it, until it did not work again.
Neil

 

[Guest]

Caution. There is a subtle difference in the modulo operation on
negative values between C90 and C99. I am not sure exactly what,
but I know it is there. Check the standard(s).

C90 allows division to round to -inf, which always results in a
nonnegative remainder. On such an implementation, the change Thad Smith
suggested would be unnecessary but harmless.