linux-kernel - Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140210191508.1a37505f@endymion.delvare>
Date:	Mon, 10 Feb 2014 19:15:08 +0100
From:	Jean Delvare <jdelvare@...e.de>
To:	Lee Jones <lee.jones@...aro.org>
Cc:	Laszlo Papp <lpapp@....org>, linux-kernel@...r.kernel.org,
	lm-sensors@...sensors.org
Subject: Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to
  contain the hwmon suffix

On Mon, 10 Feb 2014 18:01:59 +0000, Lee Jones wrote:
> > > > > Might be worth taking the opportunity to swap out these magic numbers
> > > > > now.
> > > > 
> > > > There's nothing magic about them, they tell the driver how many fans
> > > > each device supports. If you don't pass them as driver_data you'll have
> > > > to derive them from the device name in the probe function.
> > > 
> > > They're magic in that they're not easily identifiable. In the few
> > > moments that I looked at the patch I assumed they were device
> > > IDs. They should be clearly defined.
> > 
> > They could have been device IDs, some drivers do that, and that would
> > have been equally fine. driver_data can be anything. Best thing to do
> > is to document it right above the device id array if you really find it
> > confusing (I don't.) I don't know what else exactly you had in mind,
> > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> > strike me as the best coding practice.
> 
> On the contrary. Perhaps the nomenclature can be worked on a little,
> but if I saw the aforementioned defines I would have known instantly
> what was being defined without searching for co-located comments. Thus
> elevating the requirement for me to even mention it. Even when we
> use the .data element for very simple information such as device IDs
> we do so with a #define.

Right, you have a point here.

I suppose it was deemed unneeded for a ~750 lines driver nobody really
cared about. But if the driver is becoming more complex and popular
then indeed it makes sense to clean it up a little. Starting with
reordering functions to kill forward declarations ^^

-- 
Jean Delvare
Suse L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/