Bug in KBOOT code

キャンセル
次の結果を表示 
表示  限定  | 次の代わりに検索 
もしかして: 
772件の閲覧回数
fredroeber
Contributor II

Hi, I've been looking into the recently released KBOOT code. It installs as FSL_Kinetis_Bootloader_1_0_2. Just wanted to mention that the microseconds_delay function in src/microseconds/src/microseconds_sysclk.c is wrong. The function in question is:

//! @brief Delay specified time

//!

//! @param us Delay time in microseconds unit

void microseconds_delay(uint32_t us)

{

    uint32_t currentTicks = microseconds_get_ticks();

    //! The clock value in Mhz = ticks/microsecond

    uint64_t ticksNeeded = (us * s_tickPerMicrosecondMul8 / 8) + currentTicks;

    while(microseconds_get_ticks() > ticksNeeded)

    {

        ;

    }

}

The check "while(microseconds_get_ticks() > ticksNeeded)" is backwards I believe and unsafe too. It's unsafe because when the get_ticks value is in the "roll over" region and the check is done right (ie microseconds_get_ticks() < ticksNeeded) you could have cases where the test "misses" and the delay goes for a very long time. Correct code does a signed subtraction and checks that result.

It seems like this delay function isn't actually used in any of the loader code so the error isn't actually hurting anything. But having a broken function just begs problems in the future if someone goes to use the function. So I figured I would report it. I'm just starting to look at the loader code to adapt it to my project and figured I would just see what the code looked like. Doesn't give me a warm fuzzy feeling that one of the first things I happened to look at had a bug. But, hey, it's probably the only one ;-).

Just thought I'd alert the maintainers so they might fix it.

ラベル(1)
0 件の賞賛
1 解決策
601件の閲覧回数
MichaelMinnick
NXP Employee
NXP Employee

Hi Fred,

The reversed conditional test has been fixed for the next KBOOT release. Thanks for pointing it out!

Best Regards,

Mike

元の投稿で解決策を見る

0 件の賞賛
2 返答(返信)
601件の閲覧回数
fredroeber
Contributor II

Hey folks, just an update. So, I'm not used to using 64 bit values on small MCUs. The delay function uses such values in the time comparison code so I guess we wouldn't have to worry about time value roll over issues and problems with them if it used the values right. If the current_ticks value was declared uint64_t to match microseconds_get_ticks then I think the only other fix needed is to reverse the conditional test.

Fred

0 件の賞賛
602件の閲覧回数
MichaelMinnick
NXP Employee
NXP Employee

Hi Fred,

The reversed conditional test has been fixed for the next KBOOT release. Thanks for pointing it out!

Best Regards,

Mike

0 件の賞賛