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 02:33:03 UTC

[GitHub] [incubator-nuttx] btashton opened a new issue #951: libc ioctl not compatible with 64bit hardware

btashton opened a new issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951


   The iotctl function requires that `void *` `int` both be equal to `unsigned long` but in the case of 64 bits hardware like x86_64 this may not be the case. It seems like we should be able to make this work by just checking to see if `int` will fit in `unsigned long` rather than checking that it is exactly unsigned long.
   
   ```
   int ioctl(int fd, int req, ...)
   {
     unsigned long arg;
     va_list ap;
   
     /* Get the unsigned long argument.
      *
      * REVISIT:  This could be the cause of the crash down the road if the
      * actual size of the argument is anything other than sizeof(unsigned long).
      * Most small integers will be promoted to 'int'.  ARM should pass the
      * following test with all three types having sizeof(type) == 4 bytes.
      * 'float' should also be tested.  But 'long long' and 'double' are out of
      * the question!  Don't try to pass them.
      *
      * And what will happen if no third argument is passed?  In most cases,
      * this should just result in a garbage value for arg.  But you may
      * discover cases where something worse happens!
      */
   
     DEBUGASSERT(sizeof(int)        == sizeof(unsigned long) &&
                 sizeof(FAR void *) == sizeof(unsigned long));
   
     va_start(ap, req);
     arg = va_arg(ap, unsigned long);
     va_end(ap);
   
     /* Then let fs_ioctl() to the real work */
   
     return fs_ioctl(fd, req, 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] patacongo commented on issue #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623134975


   > Can we change this debug assert to `sizeof(int) <= sizeof(unsigned long) ` without causing issues?
   
   This could actually cause issues, couldn't it?  But there are a lot of issues now that we have made the variadic ioctl() the only interface.  I will submit an Issue.
   


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623151984


   Duplicate of #959


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623044574


   @patacongo Can we change this debug assert to `sizeof(int)        <= sizeof(unsigned long) ` without causing issues?


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623141375


   i just create Issue #959 to address the problems that I see and the only solution that I see.


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623151915


   I'll mark this as a duplicate so we can track it in your more detailed ticket.
   Duplicate of #959 


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623141540


   Hmmm... this is an Issue, not a PR.  I guess I didn't need to create another issue.
   


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623134975


   > Can we change this debug assert to `sizeof(int) <= sizeof(unsigned long) ` without causing issues?
   
   This could actually cause issues, couldn't it?  But there are a lot of issues now that we have made the variadic ioctl() the only interface.  I will submit an Issue.
   
   Whether it is safe or not will depending (1) the sizeof(int) and (2) on the implementation of va_arg.  If there are differences in size and if va_arg uses some byte-oriented addressing they yes it would be an issue that is is extracting an 8-byte value when the provided value is only 4-bytes, for example.  But this issue is much bigger than just that.
   


----------------------------------------------------------------
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 #951: libc ioctl not compatible with 64bit hardware

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #951:
URL: https://github.com/apache/incubator-nuttx/issues/951#issuecomment-623141540


   Hmmm... this is an Issue, not a PR.  I guess I didn't need to create another issue.  But we do need to discuss general variadic ioctl() issues as a separate topic.  I am now thinking that we should revert 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