#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 Sep 5, 2008 at 1:59:32 PM by Joel Sherrill

Owner: changed from Joel Sherrill to thomas.doerfler

comment:2 Changed on Sep 8, 2008 at 10:22:28 AM by thomas.doerfler

Resolution: fixed
Status: newclosed

comment:3 Changed on Sep 8, 2008 at 9:42:11 PM by Ralf Corsepius

Resolution: fixed
Status: closedreopened

comment:4 Changed on Sep 9, 2008 at 6:21:02 AM by thomas.doerfler

Owner: changed from thomas.doerfler to Ralf Corsepius
Status: reopenednew

comment:5 Changed on Sep 9, 2008 at 7:14:54 AM by Ralf Corsepius

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.

1) i2c_transfer_wait_poll() creates the signaling resource (poll_done_flag)
and passes it together ...

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

You can see a similar pair with the functions i2c_transfer_wait_sema() and
i2c_transfer_sema_done().

Yes, I this isn't the only place in RTEMS I consider to suffer from the same bug.

comment:6 Changed on Nov 20, 2014 at 3:43:10 AM by Chris Johns

Description: modified (diff)
Resolution: wontfix
Status: newclosed

Buttered fish.

Note: See TracTickets for help on using tickets.