linux-kernel - Re: [PATCH 3/7] [RFC] Battery monitoring class

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: <20070412165151.GA2984@zarina>
Date:	Thu, 12 Apr 2007 20:51:51 +0400
From:	Anton Vorontsov <cbou@...l.ru>
To:	Randy Dunlap <randy.dunlap@...cle.com>
Cc:	linux-kernel@...r.kernel.org, kernel-discuss@...dhelds.org,
	dwmw2@...radead.org
Subject: Re: [PATCH 3/7] [RFC] Battery monitoring class

Hello Randy,

On Wed, Apr 11, 2007 at 07:53:59PM -0700, Randy Dunlap wrote:
> On Thu, 12 Apr 2007 03:25:03 +0400 Anton Vorontsov wrote:
> > +void battery_status_changed(struct battery *bat)
> > +{
> > +	pr_debug("%s\n", __FUNCTION__);
> > +	#ifdef CONFIG_LEDS_TRIGGERS
> 
> Please don't indent preprocessor controls (ifdef/endif etc.).
...
> Place 'switch' and 'case' at the same indent level.  This prevents
> the "double-indent" for the code statements.
...
> We usually try to place a blank line between local data and code.
...
> 	space after "if"
...

Much thanks, will fix.

> > +		device_remove_file(bat->dev, &dev_attr_##name);
> > +
> > +	goto success;
> > +
> > +capacity_failed:     remove_bat_attr_conditional(current);
> > +current_failed:      remove_bat_attr_conditional(voltage);
> > +voltage_failed:      remove_bat_attr_conditional(temp);
> > +temp_failed:         remove_bat_attr_conditional(max_capacity);
> > +max_capacity_failed: remove_bat_attr_conditional(max_current);
> > +max_current_failed:  remove_bat_attr_conditional(max_voltage);
> > +max_voltage_failed:  remove_bat_attr_conditional(min_capacity);
> > +min_capacity_failed: remove_bat_attr_conditional(min_current);
> > +min_current_failed:  remove_bat_attr_conditional(min_voltage);
> > +min_voltage_failed:  remove_bat_attr_conditional(status);
> 
> I thought there was a class_remove() or something like that?
> but I'm not sure of it.

[class_]device_destroy()? Yeah, but it's exactly same thing as
[class_]device_unregister.

But this is really good question. Should I manually clean up attributes,
or sysfs will take care? I.e. whole battery directory removes, and
sysfs removes files in it, or they just leaks?

I've took worst scenario, and done removal manually (I've grep'ed in
drivers/, and almost every driver also doing so).

> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***

Thanks for comments!

-- 
Anton Vorontsov
email: cbou@...l.ru
backup email: ya-cbou@...dex.ru
irc://irc.freenode.org/bd2
-
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/