Re: [PATCH] hwmon: (core) Avoid ifdef in C source file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/13/24 20:40, Thomas Weißschuh wrote:
Hi Guenter,

On 2024-11-12 22:52:36-0800, Guenter Roeck wrote:
On 11/12/24 20:39, Thomas Weißschuh wrote:
Using an #ifdef in a C source files to have different definitions
of the same symbol makes the code harder to read and understand.
Furthermore it makes it harder to test compilation of the different
branches.

Replace the ifdeffery with IS_ENABLED() which is just a normal
conditional.
The resulting binary is still the same as before as the compiler
optimizes away all the unused code and definitions.

Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
---
This confused me a bit while looking at the implementation of
HWMON_C_REGISTER_TZ.
---
   drivers/hwmon/hwmon.c | 21 ++++++---------------
   1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 9c35c4d0369d7aad7ea61ccd25f4f63fc98b9e02..86fb674c85d3f54d475be014c3fd3dd74c815c57 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -147,11 +147,6 @@ static DEFINE_IDA(hwmon_ida);
   /* Thermal zone handling */
-/*
- * The complex conditional is necessary to avoid a cyclic dependency
- * between hwmon and thermal_sys modules.
- */
-#ifdef CONFIG_THERMAL_OF
   static int hwmon_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
   {
   	struct hwmon_thermal_data *tdata = thermal_zone_device_priv(tz);
@@ -257,6 +252,9 @@ static int hwmon_thermal_register_sensors(struct device *dev)
   	void *drvdata = dev_get_drvdata(dev);
   	int i;
+	if (!IS_ENABLED(CONFIG_THERMAL_OF))
+		return 0;
+
   	for (i = 1; info[i]; i++) {
   		int j;
@@ -285,6 +283,9 @@ static void hwmon_thermal_notify(struct device *dev, int index)
   	struct hwmon_device *hwdev = to_hwmon_device(dev);
   	struct hwmon_thermal_data *tzdata;
+	if (!IS_ENABLED(CONFIG_THERMAL_OF))
+		return;
+
   	list_for_each_entry(tzdata, &hwdev->tzdata, node) {
   		if (tzdata->index == index) {
   			thermal_zone_device_update(tzdata->tzd,

There is no dummy function for thermal_zone_device_update().
I really don't want to trust the compiler/linker to remove that code
unless someone points me to a document explaining that it is guaranteed
to not cause any problems.

I'm fairly sure that a declaration should be enough, and believe
to remember seeing such advise somewhere.
However there is not even a function declaration with !CONFIG_THERMAL.
So I can add an actual stub for it for v2.

What do you think?

You mean an extern declaration without the actual function ?
I'd really want to see that documented. It would seem rather unusual.

Besides, there are several other #ifdefs in the same file, so I am not
as much bothered about this as you are.

Thanks,
Guenter





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux