Change FIOC_MMAP, FIOC_MUNMAP and FIOC_TRUNCATE into file operation c…

[jlaitine]

…alls

• Add truncate, mmap and munmap into file_operations and remove them from ioctl definitions.

• Move truncate to be common for mountpt_operations and file_operations

• Modify all drivers to initialize the operations struct accordingly

Signed-off-by: Jukka Laitinen jukkax@ssrc.tii.ae

Summary

• IOCTLs are ment for communication from userspace application to the drivers. It doesn't make sense for an application to perform a direct IOCTL (such as FIOC_MMAP, FIOC_MUNMAP or FIOC_TRUNCATE). Application should always use posix mmap, munmap and truncate calls instead.

• For mmap, munmap and truncate there is a syscall lookup already in place in nuttx - there is no ioctl needed for the userspace-kernel communication

• Especially for "unmap", the ioctl doesn't make sense, since file_ioctl is supposed to work on a file pointer. But unmap cannot be done on a file. Even when unmapping a previously mapped "file", unmap can only work on inode, since there may not be an open file, or even a linked inode present. It must be possible to do "munmap" for a mapped file, even if the actual file is closed and/or deleted after mapping.

Impact

Cleanup of the interfaces, doesn't change the current functionality but makes it easier to add new

Testing

Inspected only.

[jlaitine]

Will gradually fix any issues in CI for this PR, lets review when it passes

[jlaitine]

I think all drivers are now fixed for the changed struct definition. @xiaoxiang781216, this is the change I proposed on the nuttx-dev list.

[xiaoxiang781216]

Sure, I will review in the weekend, thanks to provide a general solution.

[jlaitine]

forgot to revert changes to fs_rammap.* , that is now back to the original.

[jlaitine]

rebased agains current master

[xiaoxiang781216]

@jlaitine many drivers don't need change if all callbacks after ioctl are NULL.

[jlaitine]

Here I must disagree. You definitely need to change these, since they currently look like this:

  NULL,                      /* write */
   NULL,                      /* seek */
   cxd56_adc_ioctl,           /* ioctl */
   NULL                       /* poll */
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
   , NULL                     /* unlink */
#endif

Not changing it would leave misleading comments (/* poll / is not the pointer to poll, / unlink */ is not a pointer to unlink etc.).

There are really just two options:

1. delete all the trailing NULL pointers for all of these
2. add the new functions in between.

I do the 2) because this preserves the current style

[xiaoxiang781216]

Here I must disagree. You definitely need to change these, since they currently look like this:

  NULL,                      /* write */
   NULL,                      /* seek */
   cxd56_adc_ioctl,           /* ioctl */
   NULL                       /* poll */
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
   , NULL                     /* unlink */
#endif

Not changing it would leave misleading comments (/* poll / is not the pointer to poll, / unlink */ is not a pointer to unlink etc.).

There are really just two options:

1. delete all the trailing NULL pointers for all of these
2. add the new functions in between.

I do the 2) because this preserves the current style

Yes, that's why I prepare this patch: #8016 to remove the trailing NULL pointer.

[jlaitine]

Yes, that's why I prepare this patch: #8016 to remove the trailing NULL pointer.

Sure, I will update this PR as soon as your patch is merged. I see that @acassis already approved it

Removed the changes which are redundant after #8016 is merged

[xiaoxiang781216]

let's add after get_bufsize:

static size_t get_heapsize(FAR video_type_inf_t *type_inf)
{
  return type_inf->bufinf.container_size *
         get_bufsize(&type_inf->fmt[VIDEO_FMT_MAIN]);
}

and do more check here:

if (map->offset >= 0 && map->offset < get_heapsize(&priv->video_inf) &&
    map->offset + map->length <= get_heapsize(&priv->video_inf))

[acassis]

Sorry @xiaoxiang781216, I think you add it while I was hitting the merge button!

[xiaoxiang781216]

I will create a patch fix the remain issue.

[jlaitine]

Cool, thanks!

[xiaoxiang781216]

Here is the patch: #8021

[pkarashchenko]

Add FAR to void *

[pkarashchenko]

Need to fix indentations and place 1 init per line