Notice: We have migrated to GitLab launching 2024-05-01 see here: https://gitlab.rtems.org/

#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: newclosed

comment:3 Changed on 09/08/08 at 21:42:11 by Ralf Corsepius

Resolution: fixed
Status: closedreopened

comment:4 Changed on 09/09/08 at 06:21:02 by thomas.doerfler

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

comment:5 Changed on 09/09/08 at 07:14:54 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 11/20/14 at 03:43:10 by Chris Johns

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

Buttered fish.

Note: See TracTickets for help on using tickets.