Minor bug in BMI160-driver

[arurke]

In trigger_foc(), https://github.com/BoschSensortec/BMI160_driver/blob/master/bmi160.c#L6313 , the foc_status variable is not initalized with a value. Since the call to get_foc_status() can return without writing to foc_status, it will contain garbage when it is evaluated at https://github.com/BoschSensortec/BMI160_driver/blob/master/bmi160.c#L6329

[Minhwan]

Hello aruke, 

Please let me clear your issue. 

The reason is not foc_status initialziation, but failing to get data in line 6271.

foc_status would get status value from data which came from line 6271. 

Therefore, you need to check why rslt = bmi160_get_regs(BMI160_STATUS_ADDR, &data, 1, dev); doesn't work. ( line 6271).

You can check using rslt value. 

What is the for rslt? 

Thanks, 

[arurke]

My apologies, but I still believe you are not understanding the problem. I am describing a software bug in your library, not a particular issue I am experiencing. To re-iterate:

If for ANY reason (does not matter why) rstl  is not BMI160_OK on line 6271, the function we are in (get_foc_status()) will return WITHOUT doing anything to foc_status. Therefore, on line 6327, foc_status will be used without having been initialized, and the library risk crashing the microcontroller (reading uninitialized variables are undefined behavior).

I would also recommend reviewing my previous post which I believe lays it out the clearest.

[Minhwan]

Hello arurke, 

Let me clear your issue. 

Current issue is trigger_foc function doesn't work from your side for some reason. 

Based on previous our communication, the reason could be i2c or SPI communication issue because you got wrong value from rslt = bmi160_get_regs(BMI160_STATUS_ADDR, &data, 1, dev);. 

Therefore, I asked you rslt value to find the reason why you got some error value from that function. 

Please give me rslt and logic analyzer log, it can analyze the root cause. 

Thanks, 

[arurke]

I am terribly sorry, but you are not reading what I am writing.

I am not using this function, I am not having a problem currently.  This bug was found by a static code analyzer. The analyzer found that your library will lead to undefined behavior (would typically lead to a crash or worse) if certain conditions are met. Thus the solution is not avoid those conditions as you suggest, but to fix the bug in the software. The steps to reach this condition and the resulting undefined behavior is explained in my earlier comment and repeated here:

1. Line 6316: foc_status is declared but not initialized

2. Line 6326: get_foc_status() is called.

3. Line 6271: The read fails so rslt is not BMI160_OK

4. Line 6277: get_foc_status() returns. foc_status has not been assigned any value(!)

5. Line 6327: foc_status is used in comparison. This is undefined behavior(crash or worse) since foc_status has not been initialized

--------

One possible solution is to intialize foc_status to a default value at line 6316.

[Minhwan]

Hello arurke, 

Now, I clearly understand, and thanks for your several explanation. I'm really appreciated your effort. 

I will check with our apps and I will let you know. 

Thank you so much.