LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Len Brown <lenb@kernel.org>
To: Greg KH <gregkh@suse.de>
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters
Date: Tue, 5 Feb 2008 18:12:09 -0500	[thread overview]
Message-ID: <200802051812.09228.lenb@kernel.org> (raw)
In-Reply-To: <20080205221805.GC6950@suse.de>

On Tuesday 05 February 2008 17:18, Greg KH wrote:
> On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> > # cat /sys/firmware/acpi/interrupts/summary
> > pm_timer     0
> > glbl_lock    0
> > power_btn    0
> > sleep_btn    0
> > rtc          0
> > gpe00    0
...
> > gpe1F    0
> > gpe_hi    0
> > gpe_total   63
> > acpi_irq    63
> 
> Eeek!  Why?  What's wrong with individual files here?

My expectation is that this is a shell interface for debugging,
not an API for programs.  ala /proc/interrupts.

if we have 40 individual files, each with a number in it,
it is less convenient to cat the file and paste the results
into an email or bug report and have the receiver easily
see what what count goes with what file -- or is there
a version of cat that prints the file name before
the contents of each file?

I've seen 
more * | cat
but one has to wonder why an interface would be built
to make something so simple not be simple.

> What's to ensure 
> that you aren't going to overflow your buffer?

Good question.  What's to ensure we don't overflow it
when I print any other random string?
Who allocates it and how big is it?

> There's a reason we 
> don't have the seq file interface for sysfs, to prevent this very kind
> of thing...

If sysfs can't handle it, I suppose I could instead
print the needed information to the console....

> > +	/*
> > +	 * General Purpose Events
> > +	 */
> > +	for (i = 0; i < number_of_gpes; i++) {
> > +		count += sprintf(buf + count, "gpe%02X %4d\n",
> > +			i, acpi_gpe_counters[i]);
> > +	}
> 
> Nope, no error checking, fun chance of blowing the memory buffer :(

you're right, there is rumour of a future machine
with more than 32 GPEs...

> Please change the interface to not do this.
> 
> Oh, and for every new sysfs file, you need a Documentation/ABI/
> addition, please also add that.

ok.

thanks,
-Len

  reply	other threads:[~2008-02-05 23:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-05  7:30 Len Brown
2008-02-05 22:18 ` Greg KH
2008-02-05 23:12   ` Len Brown [this message]
2008-02-05 23:18     ` Greg KH
2008-02-06  1:58       ` Len Brown
2008-02-06  2:43         ` Greg KH
2008-02-06  6:33           ` Len Brown
2008-02-06  6:40             ` [PATCH] ACPI: create /sys/firmware/acpi/interrupts/ (v2) Len Brown
2008-02-07  0:16               ` Greg KH
2008-02-06  6:49             ` [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters Greg KH
2008-02-06  0:44     ` Bjorn Helgaas
2008-02-06  1:53       ` Len Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200802051812.09228.lenb@kernel.org \
    --to=lenb@kernel.org \
    --cc=gregkh@suse.de \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).