You miss an important point, it is *NOT* the possible range of values
the *TYPE* may take on.

It is the range of values the **ACTUAL*DATA** might take on that is
important here.

It's much easier to make sure bugs are not introduced in the future to make sure that things like printf respect the range of the type. Besides, it also avoids the problems of types shifting widths on different architectures. For example, in your case:


   uint32_t x;
   void funny_function( uint32_t );

   for( x = 0 ; x < 10 ; x++ ){
         printf(" X = %d, Calling funny function\n", (int)(x));
         funny_function( x ) ;
   }


Casting to int is fine as long as int is 32-bits or greater. When compiling for a smaller device though, int _could_ be 16-bit and suddenly you have truncation issues. If you care about the range of values, you should use the appropriately sized C99 type and then use the appropriate C99 printf macro to print it. Otherwise you won't know what you end up with since many of the intrinsic types in C are platform defined size.

In the above, I have several choices, (a) I could have used an "int"
variable - then *cast* it to a uint32_t at the function call (note here:
I am partial to x as a loop control variable, others like i/j/k) or -
because (b) the value will never be negative, I just choose 'uint32_t'
because it was convient.

If you know that "int" has an appropriate data range for the task, then (a) works. But you would need to verify that. You should also leave a comment stating that such a verification was done (and the date). If you chose uint32_t just for convenience, then it doesn't matter much, but you are guaranteed that using the C99 printf macro will avoid any potential truncation issues.


Here, under OpenOCD we generally:

(1) assume that all embedded openocd targets are *32bit* at most...

This is a bad assumption since it limits us from supporting various DSPs.

(2) assume all host platforms are at least 32bit host platforms

Limits us from using 16-bit processors. Not necessarily a problem, but a potentially unnecessary limitation.

(3) and thus, target addresses are generally equal to - or smaller then
the host "unsigned integers"

This is a really bad assumption. Instead we should define a new type that is the guaranteed size to hold a target address.

(4) In most, if not all of the "u32" cases, the format string specifies
"%08x" or equal.

And that is horribly wrong for "u32". %x will change size between 32- bit and 64-bit windows versions since "int" changes size. Since int is only guaranteed to be a native machine size, it could be anything. Assuming it is 32-bit or capable of holding at least 32-bits is wrong. If you are using 32-bit types, use the 32-bit printf macro.



LOG_INFO( "device id = 0x%08x (manuf 0x%03x dev 0x%02x, ver 0x %03x)",
   device_id, (device_id>>1)&0x7ff, (device_id>>12)&0xff,
(device_id>>20)&0xfff );

The format strings alone tell me that a 32bit unsigned *INTEGER* is
sufficient.
As does the math operators.


Actually they don't. The %08x indicates that you want "0" padding to ensure at least 8 hexits are printed from the int. It says very little else. Further, it isn't clear that the manufacturer, device, and version portions of the device_id are the only portions. Strictly from the code, device_id is an unknown number of bits of which 32 are known to be useful. On 32-bit systems, there are no additional bits, but on a 64-bit windows system, there are 32 additional bits that may have meaning (why else would we have printed them with %x?). In fact, this points to either an int being used where uint32_t should be used or a bug on non-32-bit systems.

--
Rick Altherr
[email protected]

"He said he hadn't had a byte in three days. I had a short, so I split it with him."
 -- Unsigned

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to