xiaoxiang781216 commented on code in PR #8000:
URL: https://github.com/apache/nuttx/pull/8000#discussion_r1059660124
##########
drivers/video/video.c:
##########
@@ -3200,6 +3198,22 @@ static int video_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int video_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
+{
+ FAR struct inode *inode = filep->f_inode;
+ FAR video_mng_t *priv = (FAR video_mng_t *)inode->i_private;
+ int ret = -EINVAL;
Review Comment:
check start/length inside bufheap
##########
drivers/video/fb.c:
##########
@@ -70,6 +70,8 @@ static ssize_t fb_write(FAR struct file *filep, FAR const
char *buffer,
size_t buflen);
static off_t fb_seek(FAR struct file *filep, off_t offset, int whence);
static int fb_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int fb_mmap(FAR struct file *filep, FAR void **addr, off_t start,
Review Comment:
start->offset match mmap usage
##########
drivers/video/video.c:
##########
@@ -3200,6 +3198,22 @@ static int video_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int video_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
+{
+ FAR struct inode *inode = filep->f_inode;
+ FAR video_mng_t *priv = (FAR video_mng_t *)inode->i_private;
+ int ret = -EINVAL;
+
+ if (addr)
+ {
+ *addr = priv->video_inf.bufheap;
Review Comment:
add start to bufheap
##########
drivers/video/video.c:
##########
@@ -197,6 +197,8 @@ static ssize_t video_read(FAR struct file *filep,
static ssize_t video_write(FAR struct file *filep,
FAR const char *buffer, size_t buflen);
static int video_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int video_mmap(FAR struct file *filep, FAR void **addr, off_t start,
Review Comment:
```suggestion
static int video_mmap(FAR struct file *filep, FAR void **addr, off_t offset,
```
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1640,38 +1643,27 @@ static off_t tmpfs_seek(FAR struct file *filep, off_t
offset, int whence)
return position;
}
-/****************************************************************************
- * Name: tmpfs_ioctl
- ****************************************************************************/
-
-static int tmpfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+static int tmpfs_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
{
FAR struct tmpfs_file_s *tfo;
- FAR void **ppv = (FAR void**)arg;
+ int ret = -EINVAL;
- finfo("filep: %p cmd: %d arg: %08lx\n", filep, cmd, arg);
DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
- /* Recover our private data from the struct file instance */
-
tfo = filep->f_priv;
DEBUGASSERT(tfo != NULL);
- /* Only one ioctl command is supported */
+ /* Recover our private data from the struct file instance */
- if (cmd == FIOC_MMAP && ppv != NULL)
+ if (addr != NULL)
Review Comment:
check offset/length inside file range
##########
fs/romfs/fs_romfs.c:
##########
@@ -617,6 +608,35 @@ static int romfs_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return -ENOTTY;
}
+static int romfs_mmap(FAR struct file *filep, FAR void **addr,
+ off_t start, size_t length)
+{
+ FAR struct romfs_mountpt_s *rm;
+ FAR struct romfs_file_s *rf;
+ int ret = -EINVAL;
+
+ /* Sanity checks */
+
+ DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
+
+ /* Recover our private data from the struct file instance */
+
+ rf = filep->f_priv;
+ rm = filep->f_inode->i_private;
+
+ /* Return the address on the media corresponding to the start of
Review Comment:
let's check offset/length inside file
##########
include/nuttx/fs/fs.h:
##########
@@ -209,13 +211,19 @@ struct file_operations
int (*close)(FAR struct file *filep);
ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
- size_t buflen);
+ size_t buflen);
Review Comment:
why change
##########
include/nuttx/fs/fs.h:
##########
@@ -209,13 +211,19 @@ struct file_operations
int (*close)(FAR struct file *filep);
ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
- size_t buflen);
+ size_t buflen);
off_t (*seek)(FAR struct file *filep, off_t offset, int whence);
int (*ioctl)(FAR struct file *filep, int cmd, unsigned long arg);
+ int (*truncate)(FAR struct file *filep, off_t length);
+ int (*mmap)(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length);
+ int (*munmap)(FAR struct task_group_s *group, FAR struct mm_map_s *map,
Review Comment:
why need struct task_group_s
##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int fb_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
+{
+ FAR struct inode *inode;
+ FAR struct fb_chardev_s *fb;
+ int ret = -EINVAL;
+
+ /* Get the framebuffer instance */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ inode = filep->f_inode;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+
+ /* Return the address corresponding to the start of frame buffer. */
Review Comment:
need check start and length in the fbmem range
##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int fb_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
+{
+ FAR struct inode *inode;
+ FAR struct fb_chardev_s *fb;
+ int ret = -EINVAL;
+
+ /* Get the framebuffer instance */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ inode = filep->f_inode;
+ fb = (FAR struct fb_chardev_s *)inode->i_private;
+
+ /* Return the address corresponding to the start of frame buffer. */
+
+ if (addr)
+ {
+ *addr = fb->fbmem;
Review Comment:
add start to fb->fbmem
##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int fb_mmap(FAR struct file *filep, FAR void **addr, off_t start,
Review Comment:
remove the space
##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int fb_mmap(FAR struct file *filep, FAR void **addr, off_t start,
Review Comment:
start->offset
##########
fs/romfs/fs_romfs.c:
##########
@@ -617,6 +608,35 @@ static int romfs_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return -ENOTTY;
}
+static int romfs_mmap(FAR struct file *filep, FAR void **addr,
+ off_t start, size_t length)
Review Comment:
```suggestion
off_t offset, size_t length)
```
##########
fs/romfs/fs_romfs.c:
##########
@@ -617,6 +608,35 @@ static int romfs_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return -ENOTTY;
}
+static int romfs_mmap(FAR struct file *filep, FAR void **addr,
+ off_t start, size_t length)
+{
+ FAR struct romfs_mountpt_s *rm;
+ FAR struct romfs_file_s *rf;
+ int ret = -EINVAL;
+
+ /* Sanity checks */
+
+ DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
+
+ /* Recover our private data from the struct file instance */
+
+ rf = filep->f_priv;
+ rm = filep->f_inode->i_private;
+
+ /* Return the address on the media corresponding to the start of
+ * the file.
+ */
+
+ if (addr && rm && rm->rm_xipbase && rf)
+ {
+ *addr = rm->rm_xipbase + rf->rf_startoffset;
Review Comment:
add offset too
##########
fs/romfs/fs_romfs.c:
##########
@@ -617,6 +608,35 @@ static int romfs_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return -ENOTTY;
}
+static int romfs_mmap(FAR struct file *filep, FAR void **addr,
Review Comment:
```suggestion
static int romfs_mmap(FAR struct file *filep, FAR void **addr,
```
##########
drivers/video/video.c:
##########
@@ -3200,6 +3198,22 @@ static int video_ioctl(FAR struct file *filep, int cmd,
unsigned long arg)
return ret;
}
+static int video_mmap(FAR struct file *filep, FAR void **addr, off_t start,
Review Comment:
```suggestion
static int video_mmap(FAR struct file *filep, FAR void **addr, off_t offset,
```
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1640,38 +1643,27 @@ static off_t tmpfs_seek(FAR struct file *filep, off_t
offset, int whence)
return position;
}
-/****************************************************************************
- * Name: tmpfs_ioctl
- ****************************************************************************/
-
-static int tmpfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+static int tmpfs_mmap(FAR struct file *filep, FAR void **addr, off_t start,
Review Comment:
```suggestion
static int tmpfs_mmap(FAR struct file *filep, FAR void **addr, off_t offset,
```
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1640,38 +1643,27 @@ static off_t tmpfs_seek(FAR struct file *filep, off_t
offset, int whence)
return position;
}
-/****************************************************************************
- * Name: tmpfs_ioctl
- ****************************************************************************/
-
-static int tmpfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+static int tmpfs_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
{
FAR struct tmpfs_file_s *tfo;
- FAR void **ppv = (FAR void**)arg;
+ int ret = -EINVAL;
- finfo("filep: %p cmd: %d arg: %08lx\n", filep, cmd, arg);
DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
- /* Recover our private data from the struct file instance */
-
tfo = filep->f_priv;
DEBUGASSERT(tfo != NULL);
- /* Only one ioctl command is supported */
+ /* Recover our private data from the struct file instance */
- if (cmd == FIOC_MMAP && ppv != NULL)
+ if (addr != NULL)
{
- /* Return the address on the media corresponding to the start of
- * the file.
- */
-
- *ppv = (FAR void *)tfo->tfo_data;
- return OK;
+ *addr = tfo->tfo_data;
Review Comment:
add offset
##########
include/nuttx/fs/fs.h:
##########
@@ -209,13 +211,19 @@ struct file_operations
int (*close)(FAR struct file *filep);
ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
- size_t buflen);
+ size_t buflen);
off_t (*seek)(FAR struct file *filep, off_t offset, int whence);
int (*ioctl)(FAR struct file *filep, int cmd, unsigned long arg);
+ int (*truncate)(FAR struct file *filep, off_t length);
+ int (*mmap)(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length);
+ int (*munmap)(FAR struct task_group_s *group, FAR struct mm_map_s *map,
+ void *start, size_t length);
/* The two structures need not be common after this point */
int (*poll)(FAR struct file *filep, struct pollfd *fds, bool setup);
+
Review Comment:
revert
##########
fs/tmpfs/fs_tmpfs.c:
##########
@@ -1640,38 +1643,27 @@ static off_t tmpfs_seek(FAR struct file *filep, off_t
offset, int whence)
return position;
}
-/****************************************************************************
- * Name: tmpfs_ioctl
- ****************************************************************************/
-
-static int tmpfs_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+static int tmpfs_mmap(FAR struct file *filep, FAR void **addr, off_t start,
+ size_t length)
{
FAR struct tmpfs_file_s *tfo;
- FAR void **ppv = (FAR void**)arg;
+ int ret = -EINVAL;
- finfo("filep: %p cmd: %d arg: %08lx\n", filep, cmd, arg);
DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
- /* Recover our private data from the struct file instance */
Review Comment:
keep this line, but remove line 1658
##########
include/nuttx/fs/fs.h:
##########
@@ -295,9 +303,14 @@ struct mountpt_operations
int (*close)(FAR struct file *filep);
ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
- size_t buflen);
+ size_t buflen);
Review Comment:
```suggestion
size_t buflen);
```
##########
include/nuttx/fs/fs.h:
##########
@@ -284,7 +292,7 @@ struct mountpt_operations
* information to manage privileges.
*/
- int (*open)(FAR struct file *filep, FAR const char *relpath,
+ int (*open)(FAR struct file *filep, FAR const char *relpath,
Review Comment:
revert the change
##########
include/nuttx/fs/fs.h:
##########
@@ -209,13 +211,19 @@ struct file_operations
int (*close)(FAR struct file *filep);
ssize_t (*read)(FAR struct file *filep, FAR char *buffer, size_t buflen);
ssize_t (*write)(FAR struct file *filep, FAR const char *buffer,
- size_t buflen);
+ size_t buflen);
off_t (*seek)(FAR struct file *filep, off_t offset, int whence);
int (*ioctl)(FAR struct file *filep, int cmd, unsigned long arg);
+ int (*truncate)(FAR struct file *filep, off_t length);
Review Comment:
why char driver need truncate callback? should we remove FIOC_TRUNCATE
directly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
