Discovred an error in Chip_IAP_ReadUID() function (from LPCOpen)

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

Discovred an error in Chip_IAP_ReadUID() function (from LPCOpen)

633 Views
lpcware
NXP Employee
NXP Employee
Content originally posted in LPCWare by cgroen on Thu Jan 14 06:23:30 MST 2016
Discovered a "small" bug in the Chip_IAP_ReadUID() function
(with severe corruption of data....)

/* Read the unique ID */
uint32_t Chip_IAP_ReadUID(uint32_t* uid)
{
uint32_t command[5], result[5];
uint32_t i;

command[0] = IAP_READ_UID_CMD;
iap_entry(command, result);

for (i=0; i<4; i++)
[color=#f00]//*(uid+i) = result[i+1]; // Not good !!![/color]
[color=#6c0]*((unsigned char*)uid+i) = result[i+1]; // Quick and dirty solution that works...[/color]

return result[0];
}

Labels (1)
0 Kudos
Reply
2 Replies

466 Views
lpcware
NXP Employee
NXP Employee
Content originally posted in LPCWare by cgroen on Wed Feb 17 00:32:35 MST 2016
Marc,
you are absolutely right!
I had my mind fixed on the id being an 32 bit value (from other families of CPUs) and was fooled into believing this by the returntype of the Chip_IAP_ReadUID().
I have changed the parameter in my code to a uint32_t xxx[4] instead!
Thanks for the heads-up!
(and I agree, it would have been better to use a struct etc as parameter to show whats going on)
0 Kudos
Reply

466 Views
lpcware
NXP Employee
NXP Employee
Content originally posted in LPCWare by MarcVonWindscooting on Tue Feb 16 14:07:13 MST 2016
Sorry: No.

I believe, the true bug of this function is the type of uid which pretends to point to a 32-bit value, but actually is a uint32_t uid[4] and had better be a structure or union. The result is 128bits, but the API fooled you into believing it's 32bit only!

Next flaw: does command have to be a uint32_t[5] and a uint32_t[1] is really not sufficient??  ;-)

One function, two reasons to stop looking at 'typical' embedded programmers' code...

Marc
0 Kudos
Reply