On Thu, 22 Dec 2016, Guenter Roeck wrote: > On 12/22/2016 04:29 AM, Julia Lawall wrote: > > > > > > On Wed, 21 Dec 2016, Guenter Roeck wrote: > > > > > Hi Julia, > > > > > > On Wed, Dec 21, 2016 at 08:39:38PM +0100, Julia Lawall wrote: > > > > > > > > > > > > On Wed, 21 Dec 2016, Guenter Roeck wrote: > > > > > > > > > Hi Julia, > > > > > > > > > > On Wed, Dec 21, 2016 at 03:05:37PM +0100, Julia Lawall wrote: > > > > > > A solution is below: the semantic patch, an explanation of the > > > > > > semantic > > > > > > patch, and the results. I have only tried to compile the results > > > > > > (make > > > > > > drivers/hwmon/). Two affected files were not considered for > > > > > > compilation: > > > > > > > > > > > > drivers/hwmon/vexpress-hwmon.o > > > > > > drivers/hwmon/jz4740-hwmon.o > > > > > > I compile tested those two patches. If possible please drop > > > vexpress-hwmon.c > > > from the patch series; the changes in that file don't add any value. > > > > > > I compile tested all files, and reviewed the patch. It all looks good. > > > Please submit the series. > > > > > > Again, thanks a lot for your help! > > > > I have sent the patches. I adjusted the semantic patch so that the > > indentation of function parameters/arguments would only change if the > > length of the function name changes. > > > > Do you think this could be of more general interest in the Linux kernel? > > Since the semantic patch works pretty well, I could add it to the > > scripts/coccinelle directory? Previously, however, I got some negative > > feedback about this change, because people felt that the new names hid the > > actual behavior, so I didn't pursue it. > > > > I do think it would add a lot of value, if for nothing else as an excellent > example > of what can be done with coccinelle. > > I actually liked the name changes. I think it is a good idea if the function > name > reflects the sysfs attribute it serves (isn't that exactly what it does, ie > its > behavior ?). But, as you have experienced, some people inadvertently did not > like > it. Given that, I am not sure if it is worth adding it to the kernel source > tree. > Maybe you could submit it as RFC so it is at least on record. > > Anyway, for SENSOR_DEVICE_ATTR(), I'll have to be a bit more flexible since > the function _will_ be reused. I'll need something like > SENSOR_DEVICE_ATTR_{RO,RW,WO}(attr, func, param) Chosen at random, static SENSOR_DEVICE_ATTR_2(sf2_point4_fan1, S_IRUGO | S_IWUSR, show_sf2_point, store_sf2_point, 4, 1); should become static SENSOR_DEVICE_ATTR_2_RW(sf2_point4_fan1, sf2_point, 4, 1); ? And the functions should be renamed with show and store at the end? > Maybe Greg would be open to something like > DEVICE_ATTR_FUNC_{RO,RW,WO}(attr,func) > to accommodate the "I want my own function name" crowd ? That would also solve > the case where the function is reused for multiple attributes. Actually, it was the DEVICE_ATTR_{RO,RW,WO} that wasn't liked. It doesn't show the exact permission numbers. The fact that not all DEVICE_ATTR uses can be changed due to function reuse is awkward, though. Greg, do you have any thoughts about that? Currently, there are around 1100 calls to DEVICE_ATTR_{RO,RW,WO}. julia -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html