lkml.org 
[lkml]   [1999]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: PATCH: PCI changes for pre-2.3.16-1
Hello,

> If you want to make life easier, just do a
>
> char * pci_slot_fmt(struct pci_dev *dev)
> {
> static char fmt[10];
>
> sprintf(fmt, "%d:%d:%d", BUS(dev), SLOT(dev), FN(dev));
> return fmt;
> }

Writing functions returning pointers to static data is just a subscription
for the magic bug of the week contest. I don't want to have to take care
of "you cannot print two slot names in one printk" type things.

> if it's the re-typing of the stuff you object to, and you just want to
> have one place to change when you want to change the way you print out
> devices. I'd certainly agree with that kind of thing.

I want to save re-typing and also to unify the format of PCI slot numbers
printed at various places in the kernel. Also, it saves a bit of space
since the cached string takes only 8 bytes which is certainly less than
what does passing of two extra parameters to a single printk take.

Also, adding a slot name to struct pci_dev seems to be the simplest
and lowest-overhead solution I see.

> And I'd even agree to caching the string if I thought it had some reason
> for it (performance, clarity, whatever), but as it stands I feel that
> calling it "name" just confuses the issue completely and brings no real
> advantage.

If you see the primary problem in "name" being confusing, I can
simply change it to "slot_name" to make the things clearer.

Have a nice fortnight
--
Martin `MJ' Mares <mj@ucw.cz> http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
"Dijkstra probably hates me." -- /usr/src/linux/kernel/sched.c

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:53    [from the cache]
©2003-2011 Jasper Spaans