zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.02k stars 6.17k forks source link

util_macro: Add macro to mark unused functions #74399

Open joerchan opened 2 weeks ago

joerchan commented 2 weeks ago

Introduction

Add a macro to that adds __unused attribute to a function unless a specific Kconfig is enabled.

Problem description

Some of the zephyr utility macros take a function as input, but may end up not referencing the function based on certain Kconfig symbols being enabled. There should be an easy way to fix this warning to avoid having to ifdef the function and everything else that function may reference.

Proposed change

Add macro: UNUSED_UNLESS (or something similar) that can be added to functions that may end up being compiled out by other utility macros.

Alternatively macro specific to the macro that can end up not referencing the function that would be add the __unused attribute when needed.

Detailed RFC

Some of the zephyr utility macros take a function as input, but may end up not referencing the function based on certain Kconfig symbols being enabled. There should be an easy way to fix this warning to avoid having to ifdef the function and everything else that function may reference. The function that ends up not being used can reference other functions that only have that as a reference, and it ends up with cascading ifdef code to find all possible combinations of a static function being referenced or not.

For example:

#ifdef CONFIG_PM_DEVICE
static int enter_dpd(const struct device *const dev)
{
    /* ... */
}
#endif /* CONFIG_PM_DEVICE */

static int exit_dpd(const struct device *const dev)
{
    /* ... */
}

#ifdef CONFIG_PM_DEVICE
static int qspi_nor_pm_action(const struct device *dev,
                  enum pm_device_action action)
{
    /* ... */
    switch (action) {
    case PM_DEVICE_ACTION_SUSPEND:
    /* ... */
        ret = enter_dpd(dev);
    case PM_DEVICE_ACTION_RESUME:
    /* ... */

        ret = exit_dpd(dev);
    /* ... */
}
#endif /* CONFIG_PM_DEVICE */

static int qspi_nrfx_configure(const struct device *dev)
{
    /* ... */
    ret = exit_dpd(dev);
    /* ... */
}

Here enter_dpd needs to be ifdef, but exit_dpd has other references. If the other reference to exit_depd is also ifdef then we need an ifdef A || B on this function.

Proposed change (Detailed)

Add macro: UNUSED_UNLESS (or something similar) that can be added to functions that may end up being compiled out by other utility macros.

#define UNUSED_UNLESS(_flag) \
    IF_DISABLED(_flag, (__unused))

Usage:

UNUSED_UNLESS(CONFIG_PM_DEVICE)
static int qspi_nor_pm_action(const struct device *dev,
                  enum pm_device_action action)

Alternatively macro specific to the macro that can end up not referencing the function that would be add the __unused attribute when needed.

#ifdef CONFIG_PM_DEVICE
#define PM_DEVICE_ACTION_FUNC
#else
#define PM_DEVICE_ACTION_FUNC __unused
#endif /* CONFIG_PM_DEVICE */

Usage:

PM_DEVICE_ACTION_FUNC 
static int qspi_nor_pm_action(const struct device *dev,
                  enum pm_device_action action)

Dependencies

None

Concerns and Unresolved Questions

Not sure where exactly the UNUSED_UNLESS define belongs.

Alternatives

RFC lists 2 alternatives.

gmarull commented 2 weeks ago

why not just use __maybe_unused, even if it does not carry any information on why it may not be used? This is used in quite a few places, eg, on regulator drivers.

henrikbrixandersen commented 2 weeks ago

why not just use __maybe_unused, even if it does not carry any information on why it may not be used? This is used in quite a few places, eg, on regulator drivers.

I think this is a clean approach as well.