alarmString.h implements alarm strings in header.

[Dirk Zimoch]

The header file alarmString.h contains definitions of global variables instead of only declarations. This is a design bug. Header files should never implement global variables. They should only declare them. Otherwise multiple .c files including this header may lead to multiple instances of the same variable and thus to linker errors.

Here, as some sort of workaround, the macro epicsAlarmGLOBAL is used in alarm.h to decide whether to include alarmString.h or not. The user must enable this macro before including alarm.h in exactly one of his source files. This concept is broken by design. Consider two independent libraries (support modules) which both want to print clear text alarm strings and thus both define epicsAlarmGLOBAL for their own use. Everything works as long as one one such library is used in an application, but using both leads to a linker error. Note that in EPICS base itself, this macro is never defined and thus the strings do not exist as a linker symbol. Thus a code using them must define epicsAlarmGLOBAL to avoid a linker error.

The better (i.e. correct) way to do this is to move the definition of the alarm strings into a .c file in EPICS base and leave only the declaration in a header. I recommend somewhere in libCom. The header alarm.h should ignore epicsAlarmGLOBAL and never include alarmString.h. This should be backward compatible, because legacy code that defines epicsAlarmGLOBAL before including alarm.h still links correctly as well as code that does not use these strings.

This bug exists already in 3.13 and still in 3.15.0. From 3.13 to 3.14, the string definition was halfheartedly moved to a separate file, but just to another header instead of a .c file.

[Andrew Johnson (anj)]

There is a branch that already does this, see lp:~anj/epics-base/alarm-strings
http://bazaar.launchpad.net/~anj/epics-base/alarm-strings/revision/12440
I had not considered making this change on the 3.14 branch, but if you think it's worth doing and that it won't break existing code I can move it to there.

[Dirk Zimoch (dirk.zimoch)]

On 11.11.2013 18:01, Andrew Johnson wrote:
> There is a branch that already does this, see lp:~anj/epics-base/alarm-strings
> http://bazaar.launchpad.net/~anj/epics-base/alarm-strings/revision/12440
> I had not considered making this change on the 3.14 branch, but if you think it's worth doing and that it won't break existing code I can move it to there.
>

Hi Andrew,

Great, I did not know that. However your approach is not backward
compatible since it requires the user to include alarmString.h directly now.

I suggest to keep the declaration of the string variables in alarm.h. So
no user code needs to be modified. (You can test this with an unmodified
tool_lib.c)

I also would appreciate a patch for 3.14.12.

Dirk

[Andrew Johnson (anj)]

You're right; I've moved the array declarations back into alarm.h so now alarmString.h is purely there for very old code and can be deprecated again.

I am still a bit reluctant to do this on the 3.14 branch though.