linux-kernel - Re: [RFC] led-class: always implement blinking

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: <1285098497.12764.18.camel@jlt3.sipsolutions.net>
Date:	Tue, 21 Sep 2010 21:48:17 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Richard Purdie <rpurdie@...ys.net>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] led-class: always implement blinking

On Tue, 2010-09-21 at 12:44 -0700, Andrew Morton wrote:
> On Fri, 20 Aug 2010 11:21:42 +0200
> Johannes Berg <johannes@...solutions.net> wrote:
> 
> > +static int led_blink_set(struct led_classdev *led_cdev,
> > +			 unsigned long *delay_on, unsigned long *delay_off)

> > +	if (*delay_on == led_cdev->blink_delay_on &&
> > +	    *delay_off == led_cdev->blink_delay_off)
> > +		return 0;
> > +
> > +	/* deactivate previous settings */
> > +	del_timer_sync(&led_cdev->blink_timer);
> > +
> > +	led_cdev->blink_delay_on = *delay_on;
> > +	led_cdev->blink_delay_off = *delay_off;

> delay_on and delay_off could have been pass-by-value rather than
> pass-by-reference?  That would clean up some gunk in callers, too.
> 
> If there was some reason for doing it with pass-by-reference then that
> reason should have been documented!

Well, this function gets assigned to led_cdev->blink_set(), which is a
function pointer that takes pass-by-reference arguments. The comment
there says:

        /* Activate hardware accelerated blink, delays are in
         * miliseconds and if none is provided then a sensible default
         * should be chosen. The call can adjust the timings if it can't
         * match the values specified exactly. */
        int             (*blink_set)(struct led_classdev *led_cdev,
                                     unsigned long *delay_on,
                                     unsigned long *delay_off);

but the software implementation doesn't adjust the timings, of course. I
suppose the "adjust the timings" was also meant to update the values.

johannes

--
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/