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 (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?
(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:

    CORE_ADDR   target_address;
    target_addr +=  1;  //  illegal
    CORE_ADDR_add( &target_addr, 1 ); // allowed

It 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 platforms

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

This line of reasoning is a red-herring.

====

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

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

See above, the "complex type: CORE_ADDR"

===

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 the

But 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.

===

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

rick> The %08x indicates that you want "0" padding to ensure at least 8 
hexits are printed from the int.  It says very little else.

Agreed.

A *VALID* alternate to the C99 format specifier is thus. as follows:

    LOG_INFO( "device id = 0x%08x ",  (unsigned int)(device_id & 
0xFFFFFFFF));

This relies upon the earlier assertion: "The basic signed/unsigned host 
integer types are at least 32bit".

-Duane.











Reason:
    At run time, target (A) might be a 32bit arm, and (B) the dsp64bit - 
adding 1 - a target specific test must be performed to *wrap* at 2^(n-1)

We are not there today, and we will not be there for a very long time.

If/when that is don




>
>> (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
> kc8...@kc8apf.net
>
> "He said he hadn't had a byte in three days. I had a short, so I split 
> it with him."
>  -- Unsigned
>

_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to