You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/05/03 16:43:33 UTC
[GitHub] [incubator-nuttx] patacongo opened a new issue #959: Issues with variadic ioctil()
patacongo opened a new issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959
## History
For many years, ioctl() was non-standard function not described in any POSIX documents. The typical prototype was:
int ioctl(int fd, int cmd, unsigned long arg);
This has numerous limitations: There could only be on problem and it assume that all types of interest could fit into an unsigned long. In particular, things like pointers to structures.
Linux implemented the variadic ioctl command a few years back:
int ioctl(int fd, int cmd, ...);
For Linux compatibility, a very limited version of this variadic ioctl was implement. It was limited because it still expected a signal argument of type unsigned long. This was _not_ a significant issue, however, because the fixed, single argument version was the default.
However, Opengroup.org has recently specified the ioctl command as the variadic form: https://pubs.opengroup.org/onlinepubs/009604599/functions/ioctl.html (although it technically only applies to a special class of STREAM devices). In order comply with this Opengroup.org requirement, the default was changed so that the variadic ioctl() is now the default form (commit c3b0848284cfb99f2254dfbcaee66c277d7d27e1).
## Problems
The problem with the use of the variadic ioctl() is that now the arguments provided to the ioctl must be explicitly cast to (unsigned long) or, if they differ in width, the extraction of the single unsigned long argument will fail. In the past, no explicit cast was required; the provided argument would be automatically cast to unsigned long, if possible.
This is further complicated by newer 64-bit architectures. sizeof(void *) make not be equal to sizeof(unsigned long).
## Solution
There is only on proper solution:
1. Remove the legacy, fixed argument ioctl() interface altogether; remove CONFIG_LIBC_IOCTL_VARIADIC
2. Do not extract the single unsigned int argument; do not call file_ioctl(). Instead, create a new os interface vioctl() which is also a system call:
int vioctl(int fd, int cmd, va_list arg);
3. Replace all implementations of ioctl() in all drivers with the variadic argument list.
4. For each ioctl command, call va_start() to get the argument. Remove all current casts of the unsigned long arg.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
btashton commented on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623144852
I'm not sure that there is an issue here now that we just make sure int fits in unsigned long.
(Normally)
For x86_64:
unsigned long is 8 bytes
int is 4 bytes
(void *) is 8 bytes
Seems like we should be fine?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623154003
But from https://pubs.opengroup.org/onlinepubs/009604599/functions/ioctl.html:
The arg argument represents additional information that is needed by this specific STREAMS device to perform the requested function. The type of arg depends upon the particular control request, but it shall be either an integer or a pointer to a device-specific data structure.
ioctl just accept one argument either an integer or pointer even it has variadic argument list. It is different from wdog which really accept the different number of arguments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623909311
https://github.com/apache/incubator-nuttx/pull/965 fix the fist two steps mention in the solution section. The last two steps isn't necessary because one and only one additional argument(either integer or pointer) can be passed to ioctl from the POSIX standard.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623154003
But from https://pubs.opengroup.org/onlinepubs/009604599/functions/ioctl.html:
The arg argument represents additional information that is needed by this specific STREAMS device to perform the requested function. The type of arg depends upon the particular control request, but it shall be either an integer or a pointer to a device-specific data structure.
ioctl just accept one argument either an integer or pointer even it has variadic argument list. It is different from wdog which really accept the different number of arguments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623149173
We went through this same discussion involving the wdog wdparm_t type some time back. There was a Rube Goldberg-ish proposal to copy all of the data into an array of type wdparm_t instead of passing varargs to avoid varying sizes of arguments.
Instead, I said that it was the caller's resposibility to cast each argument to wdparm_t. We can enforce that casting inside of the OS, but we cannot enforce that casting across the POSIX ioctl interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623148705
i think that only works for x86. It would not work in the most general case. For example, an 8-bit processor which has all of the data on a byte-aligned, 8-bit wide stack. in would be 16-bits (2 bytes) and long 32-bits (4 bytes). That would screw up the stack/parameter alignement in that case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623149173
We went through this same discussion involving the wdot wdparm_t type some time back. There was a Rube Goldberg-ish proposal to copy all of the data into an array of type wdparm_t instead of passing varargs to avoid varying sizes of arguments.
Instead, I said that it was the caller's resposibility to cast each argument to wdparm_t. We can enforce that casting inside of the OS, but we cannot enforce that casting across the POSIX ioctl interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623142416
Reverting back to the old, single argument IOCTL interface would solve all of the issues. Perhaps it is too soon to make the change of commit c3b0848? Should we revert that for now?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #959: Issues with variadic ioctl()
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #959:
URL: https://github.com/apache/incubator-nuttx/issues/959#issuecomment-623142416
Reverting back to the old, single argument IOCTL interface would solve all of the issues. Perhaps it is too soon to make the change of commit c3b0848?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org