Hi,

Here are a few examples of "broken" drivers:
 - opamp_fops
 - lcddev_fops
 - g_pca9635pw_fileops
 - g_foc_fops
 - notectl_fops
 - powerled_fops
 - g_rptun_devops
 - g_video_fops

The list is not complete. But since the "register_driver()" does not return
an error if both fops read and write pointers are NULL we are still in the
middle of discussion.

I just want to understand the algorithm to get it fixed. So let me
summarise again and if nobody has any objections I will implement the
change:
1. Zero "oflags" should be considered as illegal  and "open" call should
return "-EACCES" in this case.
2. The "register_driver()" should check if both "fops" read and write
pointers are NULL and return an error if such a situation is detected.
3. Driver register should check if "mode" value is consistent with provided
"fops" and if "read" method is NULL but "mode" contains "r" the error
should be returned. The same for write and "w" permission.
4. Update all the drivers that have both read and write methods as NULL and
add "dummy_read()" or "dummy_write" handler. Optionally in order to save
space the "fops_dummy_read" and "fops_dummy_write" can be added into
"include/nuttx/fs/fs.h" so drivers will not need to duplicate the code and
can reference a common implementation.

Please reply with your thoughts.

Best regards,
Petro

сб, 2 квіт. 2022 р. о 15:08 Gregory Nutt <[email protected]> пише:

> > By doing so we can relax check in inode_checkflags() to:
> >
> > int inode_checkflags(FAR struct inode *inode, int oflags)
> > {
> >   if ((oflags == 0) ||
> >       (oflags & O_WROK) != 0 && !inode->u.i_ops->write)
> >     {
> >       return -EACCES;
> >     }
> >   else
> >     {
> >       return OK;
> >     }
> > }
> >
> > No, this would be an error.  This would break any file, filesystem, or
> driver that really needs to support write-only behavior. In that case,
> there is a write method and no read method.
>
>  This would be a major loss of functionality.  These IOCTL-only drivers are
> degenerate cases and perhaps for them you can cut corners.  Bu, we need to
> support all existing behavior for all VFS entities.
>
> To permit opening with O_RDWR, some IOCTL drivers have dummy write methods
> too.  The loop driver again has:
>
>
> /****************************************************************************
>  * Name: loop_write
>
>  ****************************************************************************/
> static ssize_t loop_write(FAR struct file *filep, FAR const char *buffer,
>                           size_t len)
> {
>   return len; /* Say that everything was written */
> }
>

Reply via email to