Is it bad practice to declare a C function as static if it can still be executed indirectly (through a callback function)?

Question

I have a C module for an embedded system (foo.c, foo.h) that contains a function my_driver_fn() that is local in scope from an API perspective (e.g. not in foo's public header: any other code that uses its API via #include "foo.h" should not be allowed to call this function). Assume my_driver_fn() is reentrant.

However, foo uses a library libdostuff that needs to be initialized with a few user-supplied callback functions (architecture/hardware specific things) for it to work properly on any platform. In foo, my_driver_fn mentioned above would be one of the functions in question...needed by libdostuff, but not by anyone that uses foo.

Is it bad form, dangerous, disadvantageous, handicapping the compiler in any way, or undefined behavior that the compiler could capitalize on, for these callback functions (my_driver_fn() to be declared as static within foo.c? Given that its address is provided to libdostuff and it is "indirectly" called (though never directly)?

Note: I happen to be writing both foo and libdostuff, and I'm wondering whether it makes more sense for the user-supplied functions to be extern and purely resolved at link-time, or passed into libdostuff via a user-supplied callback table supplied in an initialization function (e.g. libdostuff_init(CallbackTable *user_callbacks) where CallbackTable has a function pointer that would be initialized to point to my_driver_fn)

Answer 1

In my opinion this is good practice. The static refers to the visibility of the name, and nothing else.

If the name is not needed to be used by other translation units, then marking it static reduces the risk of a clash in the "namespace" of externally visible functions .

Answer 2

It is good practice and there are no negative side-effects like poorly-defined behavior.

There are many reasons for using static, like reducing namespace pollution and avoiding accidental/intentional calls. But the main reason is actually private encapsulation design and self-documenting code - to keep functions in the module they belong, so that the outside world need not worry about how and when to call them. They should only care about the external linkage functions you have declared in the public header file.

This is for example exactly how you design ISRs in embedded systems. They should always be declared static and be placed inside the driver controlling the hardware that the ISR relates to. All communication with the ISR, including race condition protection, should be placed encapsulated in that driver. The calling application never speaks directly with the ISR and should not need to worry about re-entrancy etc. Same thing with DMA buffers.

As an example of this, I always design a cyclic timer driver for all my MCU projects, where the API to the caller lets them register simple callback functions. The timer driver then calls the callback when the timer elapses, from inside the ISR. This can then be used as a general-purpose timer for all low priority tasks that need a timer: delays, de-bouncing etc. The caller then only needs to responsibly keep the callback function minimal and specify a time in ms, but the caller doesn't know or care about the timer hardware, and the timer hardware knows nothing of the callback it executes. Loose coupling in both directions. This timer API also serves as a HAL, so you can port the code to another MCU without changing the caller code - only change the underlying driver.

Answer 3

It is good practice. It ensures the function can only be called by the code you have explicitly provided the callback to. If it had external linkage, anything could call it, whether it makes sense or not. It is clearly not impossible to abuse, but does make it more difficult to do accidentally.