#1309 closed defect
c/src/lib/libbsp/powerpc/gen5200/i2c/i2c.c: questionable volatile use-case
Reported by: | Ralf Corsepius | Owned by: | Ralf Corsepius |
---|---|---|---|
Priority: | normal | Milestone: | 4.9 |
Component: | bsps | Version: | 4.9 |
Severity: | normal | Keywords: | |
Cc: | joel.sherrill@…, sebastian.huber@… | Blocked By: | |
Blocking: |
Description (last modified by Chris Johns)
+++ This bug was initially created as a clone of Bug #1308 +++
c/src/lib/libbsp/m68k/mcf5206elite/i2c/i2c.c
contains this code:
static rtems_status_code
i2c_transfer_wait_poll(i2c_bus_number bus, i2c_message *msg, int nmsg)
{
volatile bool poll_done_flag;
rtems_status_code sc;
poll_done_flag = false;
sc = i2c_transfer(bus, nmsg, msg, i2c_transfer_poll_done_func,
(uint32_t)&poll_done_flag);
...
I.e. it passes the address of a local "volatile" variable down to other functions.
IMO, this code abuses volatile and is likely broken.
I am inclined to think, a "static bool poll_done_flags" (either global or inside
of i2c_transfer_wait_poll()) should be used instead.
c/src/lib/libbsp/powerpc/gen5200/i2c/i2c.c suffers from the same issue
Change History (6)
comment:1 Changed on 09/05/08 at 13:59:32 by Joel Sherrill
Owner: | changed from Joel Sherrill to thomas.doerfler |
---|
comment:2 Changed on 09/08/08 at 10:22:28 by thomas.doerfler
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:3 Changed on 09/08/08 at 21:42:11 by Ralf Corsepius
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:4 Changed on 09/09/08 at 06:21:02 by thomas.doerfler
Owner: | changed from thomas.doerfler to Ralf Corsepius |
---|---|
Status: | reopened → new |
comment:5 Changed on 09/09/08 at 07:14:54 by Ralf Corsepius
comment:6 Changed on 11/20/14 at 03:43:10 by Chris Johns
Description: | modified (diff) |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
Buttered fish.
Thomas, I do not agree with your analysis.
I consider this code to be "obscure, dirty hackery" and the fact it may work for you to be mere random lucky co-incidence.
No this is not what it does. It simply passes down a variable to a function.
The big question is: Where is this variable allocated?
In my understanding you are using a local nested declaration of a global variable.
This would be equivalent to using a module-global static variable.
...
Yes, I this isn't the only place in RTEMS I consider to suffer from the same bug.