Re: [PATCH 1/2] leds: make the default trigger name const
From: Trent Piepho
Date: Sun Jul 27 2008 - 22:02:32 EST
On Sun, 27 Jul 2008, Stephen Rothwell wrote:
> On Sat, 26 Jul 2008 20:08:57 -0600 Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
>> On Fri, Jul 25, 2008 at 02:01:44PM -0700, Trent Piepho wrote:
>>> The default_trigger fields of struct gpio_led and thus struct led_classdev
>>> are pretty much always assigned from a string literal, which means the
>>> string can't be modified. Which is fine, since there is no reason to
>>> modify the string and in fact it never is.
>>>
>>> But they should be marked const to prevent such code from being added, to
>>> prevent warnings if -Wwrite-strings is used and when assigned from a
>>> constant string other than a string literal (which produces a warning under
>>> current kernel compiler flags), and for general good coding practices.
>>>
>>> Signed-off-by: Trent Piepho <tpiepho@xxxxxxxxxxxxx>
>> Acked-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
>
> I would ack this as well, except I am not sure what tree this patch is
> against. In the current powerpc next tree,
It was against powerpc next from Jul 22nd, current at the time I made the
patch. It looks like that file has changed in the last few days. There is a
patch from Anton Vorontsov, "leds: mark led_classdev.default_trigger as
const", which adds const to one of the structs I modified, but doesn't get the
other one (struct gpio_led).
Then another patch from Nate Case added a new LED chip driver, and the
platform data for this driver was added as "generic led platform data", which
I don't entirely agree with. And this new struct didn't make default_trigger
const, probably because it was just copied from the gpio led platform data
with some fields removed (so it's not really that generic then, is it?).
I'll send an updated patch for current powerpc next.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/