Messages in this thread | | | Date | Tue, 27 Apr 2004 16:05:20 -0700 | From | Matthew Dharm <> | Subject | Re: [CHECKER] Implementation inconsistencies in 2.6.3 |
| |
On Tue, Apr 27, 2004 at 03:58:33PM -0700, Ken Ashcraft wrote: > > On Tue, Apr 27, 2004 at 02:47:22PM -0700, Ken Ashcraft wrote: > > > >> --------------------------------------------------------- > >> [BUG] (mdharm-usb@one-eyed-alien.net) looks like it should return count > >> instead of strlen(buf), but this is in scsiglue.c, so is it special > >> code? > >> > >> example: > >> /home/kash/linux/2.6.3/linux-2.6.3/drivers/scsi/scsi_sysfs.c:274:store_rescan_field: > >> NOTE:READ: Checking arg count [EXAMPLE=device_attribute.store-2] > >> > >> /home/kash/linux/2.6.3/linux-2.6.3/drivers/usb/storage/scsiglue.c:321:store_max_sectors: > >> ERROR:READ: Not checking arg [COUNTER=device_attribute.store-2] [fit=1] > >> [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=233] [counter=1] [z > > >> 3.20943839741638] [fn-z = -4.35889894354067] > >> > >> return sprintf(buf, "%u\n", sdev->request_queue->max_sectors); > >> } > >> > >> /* Input routine for the sysfs max_sectors file */ > >> > >> Error ---> > >> static ssize_t store_max_sectors(struct device *dev, const char *buf, > >> size_t count) > >> { > >> struct scsi_device *sdev = to_scsi_device(dev); > > > > My understanding was that I was supposed to return the number of bytes in > > the buffer that I actually used. I thought 'count' was the maximum size I > > could use. > > > > That sounds feasible. I can't find any documentation on the interface, so > I can't tell. However, there are ~233 functions that at least reference > count. Many of them are almost identical: they call sscanf or strtol to > get a length out of buf, pass that length to some subroutine, and then > unconditionally return count. This is a more representative example than > the one listed above. > > static ssize_t set_pwm_enable1(struct device *dev, const char *buf, > size_t count) > { > struct i2c_client *client = to_i2c_client(dev); > struct asb100_data *data = i2c_get_clientdata(client); > unsigned long val = simple_strtoul(buf, NULL, 10); > data->pwm &= 0x0f; /* keep the duty cycle bits */ > data->pwm |= (val ? 0x80 : 0x00); > asb100_write_value(client, ASB100_REG_PWM1, data->pwm); > return count; > }
It seems pretty shady to me to have a function which is passed a value, the entire purpose of which is to return it to the caller unchanged.
Perhaps someone on l-k can comment...
Matt
-- Matthew Dharm Home: mdharm-usb@one-eyed-alien.net Maintainer, Linux USB Mass Storage Driver
Hey Chief. We've figured out how to save the technical department. We need to be committed. -- The Techs User Friendly, 1/22/1998 [unhandled content-type:application/pgp-signature] | |