Fix to the new sysfs bin file support

I happen to notice the new binary file support in sysfs and had to take for a spin. If I understand this correct my write file will need to allocate the buffer->data, but then I have no way of freeing that memory since I don't get a release callback.

Here is a patch that:
* makes sysfs cleanup the buffer->data allocated by the attribute write functions
* fixes a bug that causes the kernel to oops when somebody attempts to write to the file.

BTW, would you be totally opposed to a patch that added open, release, and ioctl to the list of functions supported by sysfs binary files?

Another question... How would a driver know that the various write and read calls are coming from the same open, or would there be a way for a driver to make it so that only one thing can open the sysfs file at a time?
--rustyl

@@ -50,6 +50,10 @@
 		ret = count;
 	}
  Done:
+	if (buffer && buffer->data) {
+		kfree(buffer->data);
+		buffer->data = NULL;
+	}
 	return ret;
 }
 

@@ -66,7 +70,7 @@
 static int fill_write(struct file * file, const char * userbuf, 
 		      struct sysfs_bin_buffer * buffer)
 {
-	return copy_from_user(buffer,userbuf,buffer->count) ?
+	return copy_from_user(buffer->data,userbuf,buffer->count) ?
 		-EFAULT : 0;
 }
 

Hm, good point. Perhaps a better way to do it would be:

open()
allocate buffer
fill buffer

read()

	copy_to_user()

write()
copy_from_user()
sync with driver

close()
free buffer

Once the buffer is filled, then the user could read()/seek() on it to their hearts' content, without having to call the driver back.

On a write, the user would end up modifying the desired bits of the buffer, then sysfs would call the ->write() method, passing the entire buffer.

All this assumes that the buffer will not change while the file is open, but for these purposes, I think that's ok to make.

Thanks.

->ioctl() - No.

Why ->open() and ->release()? If we free the buffer in our release(), then why do you need a notification?

I don't think you could know that a ->read() came from the same process as the previous ->read(). Why would you need to know that?

Thanks,

-pat

On Thu, 2003-03-06 at 07:34, Patrick Mochel wrote:
<snip>

I was looking to see if I could use a sysfs bin file as a replacement for the char device file used by watchdog drivers which need to:
1. know when the device file is opened in order to start the timer
2. know when the device file is released in order to stop the timer if the nowayout option is not on

This also why I was asking for ioctl's.

Now I suspected that this was not the intended usage for a sysfs bin file, but I wasn't sure.

I was under the impression that some existing device drivers could use something from the file pointer to implement a session for a specific open/close. All I have ever done was to force only one process to open my device file at a time so I would not have to worry about concurrent sessions.

If I am mistaken about the ability to distinguish between different processes, then no big deal, but it would be nice to be able to force only one open at a time. (Maybe a flag in bin_attribute?)

--rustyl