On Mon, Dec 13, 2010 at 10:51:32AM -0500, Jean Delvare wrote: > On Mon, 13 Dec 2010 07:20:56 -0800, Guenter Roeck wrote: > > On Mon, Dec 13, 2010 at 07:58:42AM -0500, Jean Delvare wrote: > > > Reference: http://www.lm-sensors.org/ticket/2378 > > > > > > This is a candidate patch to let libsensors compute MAX_SUBFEATURES > > > dynamically. This should avoid accidental overflows when we add new > > > subfeatures. > > > > > > An alternative is to keep it a constant and add code to check for > > > overflows. I had a patch ready, but if we are going to add code, I'd > > > rather add code that get things right than code which only spots when > > > things are wrong. > > > > > > Then we can discuss what do to with the other constants. > > > > > > MAX_MAIN_SENSOR_TYPES and MAX_OTHER_SENSOR_TYPES could be easily > > > computed in the same loops which now compute MAX_SUBFEATURES. It's only > > > a few code lines to add. OTOH it can be discussed whether they are > > > worth the runtime cost, given that adding a new feature is a rare > > > event, so we should be able to deal with it. I would like to hear > > > opinions about this. > > > > > Added runtime cost is really negligible, so that isn't really an argument for me. > > But it is a really rare event, so it doesn't seem to be worth the effort - at least > > as long as compilation fails if the limit is exceeded. > > Unfortunately, no, the compilation doesn't fail, and this is the > problem. A bug would lead to an overflow at run-time. > > Adding a run-time check preventing the overflow (and basically ignoring > the new features) could be done. But then again, if we add code, we > might as well add code which computes things dynamically, rather than > code to work around our mistakes. > > At the moment, when one adds a entry to enum sensors_feature_type in > sensors.h, he or she has to think of increasing MAX_MAIN_SENSOR_TYPES > or MAX_OTHER_SENSOR_TYPES in sysfs.c. We could put a note in sensors.h > about it, but OTOH I don't like to mention internal implementation > details in a public header file. > > But looking at sensors.h... maybe I have an idea how to solve this > elegantly. Let me give it a try. > If that doesn't work, or ends up being complicated, I'd suggest to go with runtime detection after all. Or, in other words, KISS should be the deciding factor. Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
