BareConductive/mpr121

Wire.begin() in constructor may cause crash due to undefined contruction order of global objects

Opened this issue · 1 comments

euphi commented

As stated in PR #6 the constructor of MPR121_t may crash. (PR #6 states this for ESP8266, but the issue is valid generally).

The C++ standard gives no information about initialization order of global or static objects in different compilation units.

However, the MPR121 object is global, so the constructor is called at startup (before main(), so before arduino's setup()). The constructor then calls Wire.begin(), but there is no guarantee that the global Wire object has been constructed yet.

Solutions:

a) Lower initialization priority of MPR121 by using the gcc-extension __attribute__ ((init_priority (*prio*)))
b) Increase initialization priority of Wire (must be done in the Arduino library)
c) Don't reference any objects that do not belong to MPR121_t in the constructor.

For now I would prefer solution c), because there is no real need to call Wire.begin(). People should call Wire.begin() in their own setup()-code.

Background:

The problem with this issue is that it is caused by undefined behavior. Odds are good that the current sources compile and run fine without any problems, because Wire will be constructed before MPR121. However, subtle changes somewhere else in the code can change the construction order and lead to the problem. In my case it seems that a changes somewhere in the arduino framework triggered the problem.

I would like to admit an extension to Solution c) as i would like to implement this Library in one of my Projects.
In that Project i have to use TwoWire with specific SDA and SCL Lines.
So i would like to call mpr121.begin(MPR121_I2C_ADR, MPR121_TTH, MPR121_RTH, MPR121_IRQ_PIN, &i2cBus) in setup()