On Jun 20, 2009, at 6:16 AM, Duane Ellis wrote:
rick> For example, in your case: duane> uint32_t x; duane> void funny_function( uint32_t ); duane> duane> for( x = 0 ; x < 10 ; x++ ){ duane> printf(" X = %d, Calling funny function\n", (int)(x)); duane> funny_function( x ) ; duane> } rick> Casting to int is fine as long as int is 32-bits or greater.I assert that all OpenOCD hosts are: at least 32bit basic machines, they may be larger.===== snip =====rick> 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.I assert (a) OpenOCD requires that all host basic integer types are at least 32bits *AND*
I assert OpenOCD is a portable code base and such assumptions about data types are invalid.
I assert (b) the following example is *self*evident* and does not require a "verification comment with date"uint32_t foo;printf(" The value we care about here is: %d\n", (int)( foo & 0x0ff ) );True or false?
In this limited case where a mask is used as part of the argument to printf is fine since the data range is clearly expressed as part of the printf. Of course, the choice of uint32_t might have been poor to begin with. If the original data required between 17 and 32 bits then uint32_t is appropriate. If the data was guaranteed to have a more limited range (perhaps by the same mask), then using an int or unsigned int to begin with would be more appropriate. Remember, if you use the correct data type for the range of values required, the compiler has less work to do. In this contrived case, the compiler would need to do a 64-bit promotion on the argument if compiled on 64- bit windows even though the data is guaranteed to be within the 8bit range. Either a cast to unsigned char and %hhu or a cast to uint8_t and PRIu8 is more appropriate.
(One could have used 'unsigned int' and '%u' instead). ==== duane> Here, under OpenOCD we generally:duane> (1) assume that all embedded openocd targets are *32bit* at most...rick> This is a bad assumption since it limits us from supporting various DSPs.To fix this, requires that we introduce a "CORE_ADDR" type - much like GDB has internally, but in a far more complex way. Remember: OpenOCD is 'target agnostic' - ie: Pic32 - is built in - same as ARM - same as Future64Bit - thus CORE_ADDR will be a complex dynamic type with specialized addition and subtraction routines, that are "current target aware" for example:
If doesn't _need_ to be that complex. You can simply choose a large enough data type to encapsulate the largest target address size. That should be hidden behind a typedef, however, so it can be changed later. Then each target is responsible for validating the target address range when committing it to the hardware. In practice this works reasonably well. (Yes, I've done exactly this for tools that analyze data from various bitwidth architectures)
CORE_ADDR target_address; target_addr += 1; // illegal CORE_ADDR_add( &target_addr, 1 ); // allowedIt becomes more complex when/where the target has a *true* Harvard architecture, and/or non-linear escoterica memory (while *ARM* is generally Harvard, the majority of implementations make that go away)==== duane> (2) assume all host platforms are at least 32bit host platformsrick> Limits us from using 16-bit processors. Not necessarily a problem, but a potentially unnecessary limitation.This line of reasoning is a red-herring.
No, it's assuming portability rather than a specific set of architectures. If someone wants to run OpenOCD on a 16-bit processor, it should still work unless we happened to have introduced bugs in our use of the type system. By using the C99 types and printf macros, it doesn't matter what the underlying size of 'int' is, uint32_t will always be 32-bits.
=== duane 4) In most, if not all of the "u32" cases, the format string duane> specifies "%08x" or equal.rick> And that is horribly wrong for "u32". %x will change size between 32-bit and 64-bit windows versions since "int" changes size.I agree it is *WRONG* when the value represents a "TARGET_CORE_ADDR" type. But this has *NOTHING* to do with theBut otherwise it does not.For example: A target 32bit value (perhaps the contents of memory) the resulting value will never take on any value larger then 32bits on the host. Thus, any host side calculation must be performed with host type of least 32bits *UNTIL* the sufficiently interesting component of that value has been extracted.At that point, the author of the code may choose to use any other sufficiently wide enough variable type to hold "the interesting component".I again assert all openocd hosts are at least 32bit (or larger) basic machines. Thus, I assert that a host side cast to "int" or "unsigned int" is at least a 32bit value and in many cases is sufficient for the majority of printf() like values, TARGET_CORE_ADDR - the exception.
As I've pointed out time and time again, 'int' is not guaranteed to be of any particular size. Assuming otherwise limits portability for no reason. If you need a 32-bit value, use the data type guaranteed to give you 32-bits regardless of platform. 'int' is generally something to avoid unless you really don't care what type it is. That is limited to cases such as loops over small integer values (e.g. 0 to 10) which will fit in the smallest C data type. The author of a piece of code can always choose to use a larger type than strictly necessary, but that doesn't mean they should choose a type that can be any size.
-- 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
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
