Pat -
On Mon, Aug 05, 2002 at 12:17:13PM -0700, Patrick Mochel wrote:
> I've also created a macro[1] for defining device attributes, that goes
> like this:
>
> DEVICE_ATTR(name,"strname",mode,show,store);
Do you any plans to simplify the show or store interfaces?
We have the same interface and problem with /proc read and write.
Right now, to do it correctly (for short read buffers, seeks, or long strings
being output) using sprintf() is very painful, see sg.c for example, and its
proc read interfaces. It took me forever to realize that the code is
re-writing the same data into a buffer.
Passing a single page or two (4k to 8k buffer), with no offset, and letting
the driverfs_read_file fill buf might be OK, but breaks seeks (and short
buffer usage), but at least the show/restore functions would be less likely
to be broken. Limiting the offset to a fit in a page might help.
If the show and store interfaces could return a pointer, lengths, and
a specifier ("%s", "%d", etc.), that might be pretty simple, and would
allow for correct offset and overflow checks.
Most of the current show interfaces are broken for a short buffer or seek,
and they are being copied to create new interfaces, example usage:
[patman@elm3a50 linux-2.5.29-p1]$ cat /devices/root/pci0/00:0f.2/name
PCI device 1166:0220
[patman@elm3a50 linux-2.5.29-p1]$ dd if=/devices/root/pci0/00:0f.2/name of=/tmp/xx bs=1 count=10 ; cat /tmp/xx ; echo
1+0 records in
1+0 records out
P
I changed the scsi_scan.c scsi_device_type_read(), and got it working
OK, it does not check for overflows (snprintf does not work, since we
do not pass an offset; maybe if a snoffprintf existed it might help),
but this does work for seeks (or short reads):
static ssize_t scsi_device_type_read(struct device *driverfs_dev, char *page,
size_t count, loff_t off)
{
struct scsi_device *sdev = to_scsi_device(driverfs_dev);
ssize_t size;
if ((sdev->type > MAX_SCSI_DEVICE_CODE) ||
(scsi_device_types[(int)sdev->type] == NULL))
size = sprintf(page, "Unknown\n");
else
size = sprintf(page, "%s\n",
scsi_device_types[(int)sdev->type]);
if (off >= size)
/*
* Assumed complete.
*/
return 0;
return size;
}
For the above to function I also had to change:
===== fs/driverfs/inode.c 1.20 vs edited =====
--- 1.20/fs/driverfs/inode.c Fri Jul 26 12:58:50 2002
+++ edited/fs/driverfs/inode.c Tue Jul 30 14:01:46 2002
@@ -311,7 +311,7 @@
} else if (len > count)
len = count;
- if (copy_to_user(buf,page,len)) {
+ if (copy_to_user(buf,page+*ppos,len)) {
retval = -EFAULT;
break;
}
-- Patrick Mansfield
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Aug 07 2002 - 22:00:29 EST