Bug in Imagecraft AVR?

Hi all (and Richard, if you're here :-) )

I see some strange behaviour of the ICC AVR compiler. In the following code:

char i,r;

for (i=0;i 0) { do something.... } }

I expect the for loop to break when r is negative. However, it does not and the loop is executed 5 times, no matter the returned result of get_data_field().

Is the compiler wrong or my understanding of the effects of the break-statement.

Meindert

Reply to
Meindert Sprang
Loading thread data ...

Seems the type 'char' is unsigned, isn't it?

Vadim

Reply to
Vadim Borshchev

I've always mistrusted the 'break' function---seems too much like BASIC to me! i would code the function more like:

#define MAXFIELD 5 for (i=0;i 0) { do something.... } }

About the only only place I use 'break' is after the case elments in a switch statement.

It would be interesting to see what assembly language is being generated by your compiler for this code. Perhaps there is an obvious error in the generated object code.

Mark Borgerson

Reply to
Mark Borgerson

Another thought: Is it possible that get_data_field is returning zero?

Mark Borgerson

Reply to
Mark Borgerson

No. char is signed.

Meindert

Reply to
Meindert Sprang

What happens for r==0?

No. Plain char, signed char, and unsigned char are three distinct types. It is implementation-defined as to whether plain char is signed or not. Most 8-bit compilers I'm familiar with make it unsigned by default.

If you want a small int, use signed char. Typedef it if the name bugs you. I use S8 or SI8

Never use plain char for anything. It's useless, even dangerous. Particularly unsuitable for character data.

Regards,

-=Dave

--
Change is inevitable, progress is not.
Reply to
Dave Hansen

Yes, sometimes. But I found the culprit (with apologies to Vladimir): a char type in unsigned in ICC AVR. So I have to explicitly declare 'signed char get_data_field()' So now it works OK.

Meindert

Reply to
Meindert Sprang

It's time for some old-fashioned debugging. You are assuming that char is signed, which may be true for your compiler, but needn't be for C in general (it is good practice to type the variable signed char, which will force it to be signed on any conforming compiler). What is the return type for get_data_field()? If it is int and the return value is outside the range of r, you could get a false indication.

I suggest looking at the generated code. You may be able to have the compiler generate assembly output or you can look at disassembled code. In either case, verify the code for the first test on r.

It may be that the value returned in r isn't what you think it is. You can use a simulator, ICE, or other means to inspect the returned data. For debugging realtime applications on the fly I have inserted code to generate pulse patterns on a spare pin at certain critical points in the code.

Thad

Reply to
Thad Smith

And so I have noticed :-(....

When I write in C builder, char is always signed. In my older versions of '51 compiler, it was signed. Even in my Z80 compiler. Sigh..... Thanks for the hint though.

Meindert

Reply to
Meindert Sprang

I'd use a loop that allows me to do checks, eg a while loop.

You're aware that imagecraft have their own list servers to answer questions related to the ICCAVR ? Available at the imagecraft website.

Rene

--
Ing.Buero R.Tschaggelar - http://www.ibrtses.com
& commercial newsgroups - http://www.talkto.net
Reply to
Rene Tschaggelar

... Yes, I am still here :-) and yes, the problem is that "char" is unsigned.

-- // richard

formatting link

Reply to
Richard F. Man

No, the problem is the programmer _assumed_ it was signed.

-- Grant Edwards grante Yow! Where's th' DAFFY at DUCK EXHIBIT?? visi.com

Reply to
Grant Edwards

"Meindert Sprang" wrote in news: snipped-for-privacy@news.nb.nu:

Although the folks in comp.lang.c don't approve of this, I find it has saved me many a time in my embedded work with C compilers. Once I include this file I never use C's char, int, short, etc., only these base types.

#ifndef fixed_types_h_included #define fixed_types_h_included

/* fixed_types.h

** ** Programmer is responsible for determining if compiler ** and architecture support these types and bit widths. ** ** U = Unsigned integer type ** S = Signed integer type ** F = Floating point type ** ** 8,16,32,64 indicates the bit width of the type */ typedef unsigned char U8; typedef signed char S8; typedef unsigned short U16; typedef signed short S16; typedef unsigned long U32; typedef signed long S32; typedef float F32; typedef double F64;

#endif /* fixed_types_h_included */

HTH.

--
- Mark ->
--
Reply to
Mark A. Odell

Well, it does not make much difference which loop you use. Even the for loop does a check. Instead of usgin the obvious

for (i=0;i You're aware that imagecraft have their own list servers to answer questions

Didn't know that. I'll take a look there. Thanks.

Meindert

Reply to
Meindert Sprang

Hi Richard,

Yes, I have noticed it. And read it in the help file when I specifically looked for it. Thanks.

Meindert

Reply to
Meindert Sprang

unsigned.

Yes. But it was an assumption based on experience. All compilers I have previously used treated char as signed unless an option was set. But you learn something everyday :-)

Meindert

Reply to
Meindert Sprang

Here's my usage, which I recommend:

signed char for 8-bit (or more) signed values unsigned char for 8-bit (or more) unsigned values char for actual characters.

This is portable to any conforming C compiler.

Thad

Reply to
Thad Smith

I thought the same but I just looked in K&R and they specifically state that the sign of char is _not_ defined.

Actually I always do a typedef of unsigned char to byte which seems much more sensible to me for an embedded system.

Mike Harding

Reply to
Mike Harding

They're the worst kind, because you don't much think about them.

I've decided it's time to start using the C99 standard names when I want to specify exactly how many bits to use:

8-bit: int8_t uint8_t 16-bit: int16_t uint16_t 32-bit: int32_t uint32_t 64-bit: int64_t uint64_t

If your compiler doesn't suppor them via stdint.h, you can certainly make your own typedefs.

And here's a clever hack from

formatting link
that will automagically generate a compile time error if the typedefs aren't right:

static union { char int8_t_incorrect[sizeof( int8_t) == 1]; char uint8_t_incorrect[sizeof( uint8_t) == 1]; char int16_t_incorrect[sizeof( int16_t) == 2]; char uint16_t_incorrect[sizeof(uint16_t) == 2]; char int32_t_incorrect[sizeof( int32_t) == 4]; char uint32_t_incorrect[sizeof(uint32_t) == 4]; };

--
Grant Edwards                   grante             Yow!  Where's th' DAFFY
                                  at               DUCK EXHIBIT??
                               visi.com
Reply to
Grant Edwards
[...]

From a practical standpoint, I would suggest using unsigned char for "actual" characters. In all cases, the char value of a member of the execution character set cast to unsigned char will have the same representation (and vice-versa), so there's no real downside, even for standard library functions expecting char or char*. And it prevents errors like the following:

A number of years ago, I had a system with a custom terminal for the user interface. The terminal had a set of function keys, and would return 0xF1 for F1, 0xF2 for F2, etc. I learned the hard way that the following is an infinite loop if char is signed (functions simplified for the sake of the example):

#define EXIT_KEY 0xF1 char get_key(void){return EXIT_KEY;} void process(char k){printf("key is %02x\n",k);}

char key; do{ key = get_key(); process(key); } while (key != EXIT_KEY);

The example might be familiar to some of you. I've posted it before. Several times. ;-)

Regards,

-=Dave

--
Change is inevitable, progress is not.
Reply to
Dave Hansen

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.