Place the miscdevice in the file's private_data to be used by the file operations.
Previously, this was done only when an open() operation had been registered.
But it is also useful in the other file operations, so we pass it on unconditionally.
Signed-off-by: Tom Van Braeckel <[email protected]>
---
drivers/char/misc.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index ffa97d2..c3681fb 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -140,10 +140,16 @@ static int misc_open(struct inode * inode, struct file * file)
goto fail;
}
+ /*
+ * Place the miscdevice in the file's
+ * private_data so it can be used by the
+ * file operations, including f_op->open below
+ */
+ file->private_data = c;
+
err = 0;
replace_fops(file, new_fops);
if (file->f_op->open) {
- file->private_data = c;
err = file->f_op->open(inode,file);
}
fail:
--
1.9.1
On Thu, Dec 04, 2014 at 10:08:15PM +0100, Tom Van Braeckel wrote:
> Place the miscdevice in the file's private_data to be used by the file operations.
>
> Previously, this was done only when an open() operation had been registered.
> But it is also useful in the other file operations, so we pass it on unconditionally.
>
> Signed-off-by: Tom Van Braeckel <[email protected]>
> ---
> drivers/char/misc.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index ffa97d2..c3681fb 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -140,10 +140,16 @@ static int misc_open(struct inode * inode, struct file * file)
> goto fail;
> }
>
> + /*
> + * Place the miscdevice in the file's
> + * private_data so it can be used by the
> + * file operations, including f_op->open below
> + */
> + file->private_data = c;
> +
> err = 0;
> replace_fops(file, new_fops);
> if (file->f_op->open) {
> - file->private_data = c;
> err = file->f_op->open(inode,file);
> }
These braces aren't needed anymore, right?
Also, what is this now going to break? :)
thanks,
greg k-h
> >
> > + /*
> > + * Place the miscdevice in the file's
> > + * private_data so it can be used by the
> > + * file operations, including f_op->open below
> > + */
> > + file->private_data = c;
> > +
> > err = 0;
> > replace_fops(file, new_fops);
> > if (file->f_op->open) {
> > - file->private_data = c;
> > err = file->f_op->open(inode,file);
> > }
>
> These braces aren't needed anymore, right?
>
> Also, what is this now going to break? :)
>
> thanks,
>
> greg k-h
Thanks for the fast feedback!
Good point, so I made a list of all drivers that use private_data (find drivers/ -iname "*.c" -exec grep -l private_data {} \; | sort -u) and manually verified that all of them either register an open() operation, do not use the misc subsystem, or matched because of vm_private_data.
>From this investigation, I conclude that it does not break anything.
I also removed the unnecessary curly braces. Please find the new patch below.
misc: pass miscdevice through file's private_data
Place the miscdevice in the file's private_data to be used by the file operations.
Previously, this was done only when an open() operation had been registered.
But it is also useful in the other file operations, so we pass it on unconditionally.
All drivers that use private_data and misc_register() were checked to ensure they register an open() operation,
in order to verify that this improvement does not break any existing driver that uses the misc subsystem.
Signed-off-by: Tom Van Braeckel <[email protected]>
---
drivers/char/misc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index ffa97d2..d6445de 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -140,12 +140,17 @@ static int misc_open(struct inode * inode, struct file * file)
goto fail;
}
+ /*
+ * Place the miscdevice in the file's
+ * private_data so it can be used by the
+ * file operations, including f_op->open below
+ */
+ file->private_data = c;
+
err = 0;
replace_fops(file, new_fops);
- if (file->f_op->open) {
- file->private_data = c;
+ if (file->f_op->open)
err = file->f_op->open(inode,file);
- }
fail:
mutex_unlock(&misc_mtx);
return err;
--
1.9.1
On Fri, Dec 05, 2014 at 12:01:03AM +0100, Tom Van Braeckel wrote:
> > >
> > > + /*
> > > + * Place the miscdevice in the file's
> > > + * private_data so it can be used by the
> > > + * file operations, including f_op->open below
> > > + */
> > > + file->private_data = c;
> > > +
> > > err = 0;
> > > replace_fops(file, new_fops);
> > > if (file->f_op->open) {
> > > - file->private_data = c;
> > > err = file->f_op->open(inode,file);
> > > }
> >
> > These braces aren't needed anymore, right?
> >
> > Also, what is this now going to break? :)
> >
> > thanks,
> >
> > greg k-h
>
> Thanks for the fast feedback!
>
> Good point, so I made a list of all drivers that use private_data (find drivers/ -iname "*.c" -exec grep -l private_data {} \; | sort -u) and manually verified that all of them either register an open() operation, do not use the misc subsystem, or matched because of vm_private_data.
>
> >From this investigation, I conclude that it does not break anything.
>
> I also removed the unnecessary curly braces. Please find the new patch below.
>
>
> misc: pass miscdevice through file's private_data
>
> Place the miscdevice in the file's private_data to be used by the file operations.
>
> Previously, this was done only when an open() operation had been registered.
> But it is also useful in the other file operations, so we pass it on unconditionally.
>
> All drivers that use private_data and misc_register() were checked to ensure they register an open() operation,
> in order to verify that this improvement does not break any existing driver that uses the misc subsystem.
>
> Signed-off-by: Tom Van Braeckel <[email protected]>
> ---
> drivers/char/misc.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
Please resend this in a format I can apply it in (standalone, lines
properly wrapped, etc.)
thanks,
greg k-h
Make the miscdevice accessible through the file's private_data.
Previously, this was done only when an open() operation had been registered.
But it's also useful in other file operations so we pass it on unconditionally.
All drivers that use private_data and misc_register() were checked to ensure
they register an open() operation in order to verify that this improvement
does not break any existing driver that uses the misc subsystem.
Signed-off-by: Tom Van Braeckel <[email protected]>
---
drivers/char/misc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index ffa97d2..d6445de 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -140,12 +140,17 @@ static int misc_open(struct inode * inode, struct file * file)
goto fail;
}
+ /*
+ * Place the miscdevice in the file's
+ * private_data so it can be used by the
+ * file operations, including f_op->open below
+ */
+ file->private_data = c;
+
err = 0;
replace_fops(file, new_fops);
- if (file->f_op->open) {
- file->private_data = c;
+ if (file->f_op->open)
err = file->f_op->open(inode,file);
- }
fail:
mutex_unlock(&misc_mtx);
return err;
--
1.9.1