On Mon, 12 Oct 2009 18:02:12 +0100, Jonathan Cameron wrote: > Jean Delvare wrote: > > The name size really isn't an issue. You won't notice 16 bytes instead > > of 9. And it's not like we'll have millions of these devices. > > I agree, it's merely a question of picking some suitable limit. IDR's > on a 64 bit box will do something in the ball park of 2e18 which might > be an excessive limit ;) I'll leave this be for next version on basis In all honesty, I don't expect als device names of ~30 chars to cause a memory shortage to any machine any time soon. I'm not claiming that we are likely to see machines with 2e18 light sensors any time soon either. All I'm saying is that arbitrary limits should be avoided when possible. > (...) > Counter argument placed (cc'd Pavel and Corentin for this point) > is that having a generic name, e.g. hwmon0 and a name field in sysfs > is superfluous when we can combine the two. Counter counter argument is that this then makes it impossible to have different subtypes and differentiate by name. See the input subsystem for an example: they have input%d and event%d devices, for two different purposes. It's very easy for both users and user-space software to see what is what with this naming scheme. Other examples of this include video4linux and sound classes. As a matter of fact, looking at all classes that are in use on my machine, I can't see any class which leaves device naming up to its implementing drivers (except apparently mem and misc, but you'll have to agree that these are very special classes.) As a summary, I can't see any reason to come up with a new way to handle class device names in the als class, when all other classes around have already agreed on a standard approach. > > (...) > > The length is fine until someone needs more sensors, changes > > TSL2550_DEV_MAX to 16, and then the code crashes and nobody knows why. > > You can't assume that other developers won't touch your code. Thus it > > is much better to handle a given limitation in a single place. Here you > > can simply check the value returned by snprintf() and then you know if > > your name was correctly set. > > > > Again, not that it matters, because the name buffer should simply be > > large enough that it always fits. > > That's awfully big if we don't place some arbitrary limit on number of Where "awfully big" is like 30 bytes. Wow, I'm frightened :) > devices. I'm happy to pick another one, but one is needed. Clearly the > code can be made more resilient to the sort of change you mention as well > and I'll do that if this isn't moved into the core. Whatever limit you end up with, it should be set by the buffer size, rather than a separate constant. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
