diff mbox series

fixincludes: vxworks: remove unnecessary parentheses in ioctl wrapper macro

Message ID 20180629094700.5445-1-rv@rasmusvillemoes.dk
State New
Headers show
Series fixincludes: vxworks: remove unnecessary parentheses in ioctl wrapper macro | expand

Commit Message

Rasmus Villemoes June 29, 2018, 9:47 a.m. UTC 
The rationale for the fixinclude ioctl macro wrapper is, as far as I can
tell (https://gcc.gnu.org/ml/gcc-patches/2012-09/msg01619.html)

  Fix 2: Add hack for ioctl() on VxWorks.

  ioctl() is supposed to be variadic, but VxWorks only has a three
  argument version with the third argument of type int.  This messes up
  when the third argument is not implicitly convertible to int.  This
  adds a macro which wraps around ioctl() and explicitly casts the third
  argument to an int.  This way, the most common use case of ioctl (with
  a const char * for the third argument) will compile in C++, where
  pointers must be explicitly casted to int.

However, we have existing C++ code that calls the ioctl function via

  ::ioctl(foo, bar, baz)

and obviously this breaks when it gets expanded to

  ::(ioctl)(foo, bar, (int)(baz))

Since the GNU C preprocessor already prevents recursive expansion of
function-like macros, the parentheses around ioctl are unnecessary.

Incidentally, there is also a macro sioIoctl() in the vxworks sioLib.h
header that expands to

  ((pSioChan)->pDrvFuncs->ioctl (pSioChan, cmd, arg))

which also breaks when that gets further expanded to

  ((pSioChan)->pDrvFuncs->(ioctl) (pSioChan, cmd, (int)(arg)))

This patch partly fixes that issue as well, but the third argument to
the pDrvFuncs->ioctl method should be void*, so the cast to (int) is
slightly annoying. Internally, we've simply patched the sioIoctl macro:

  (((pSioChan)->pDrvFuncs->ioctl) (pSioChan, cmd, arg))

==changelog==

fixincludes/

	* inclhack.def (vxworks_ioctl_macro): Remove parentheses from
	expansion of ioctl macro.
	* fixincl.x: Regenerate.
---
 fixincludes/inclhack.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Olivier Hainque Sept. 3, 2018, 9:45 a.m. UTC | #1 
Hi Rasmus,

> On 29 Jun 2018, at 11:47, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:

> -        "#define ioctl(fd, func, arg) (ioctl)(fd, func, (int)(arg))\n";
> +        "#define ioctl(fd, func, arg) ioctl(fd, func, (int)(arg))\n";

ok by me, thanks.
Bruce Korb Sept. 3, 2018, 3:25 p.m. UTC | #2 
On Mon, Sep 3, 2018 at 2:46 AM Olivier Hainque <hainque@adacore.com> wrote:
> > -        "#define ioctl(fd, func, arg) (ioctl)(fd, func, (int)(arg))\n";
> > +        "#define ioctl(fd, func, arg) ioctl(fd, func, (int)(arg))\n";
>
> ok by me, thanks.

Shouldn't this qualify as "trivial"? :)
Olivier Hainque Sept. 3, 2018, 3:29 p.m. UTC | #3 
> On 3 Sep 2018, at 17:25, Bruce Korb <bkorb@gnu.org> wrote:
> 
> On Mon, Sep 3, 2018 at 2:46 AM Olivier Hainque <hainque@adacore.com> wrote:
>>> -        "#define ioctl(fd, func, arg) (ioctl)(fd, func, (int)(arg))\n";
>>> +        "#define ioctl(fd, func, arg) ioctl(fd, func, (int)(arg))\n";
>> 
>> ok by me, thanks.
> 
> Shouldn't this qualify as "trivial"? :)

Probably, though I kept thinking there might
have been a particular reason, which I couldn't
grasp, why the parens had been introduced back
then.

Thanks for chiming in :-)
diff mbox series

Patch

diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index c1f5a13eda4..f7d2124ba74 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -4902,7 +4902,7 @@  fix = {
 
     c_fix       = format;
     c_fix_arg   = "%0\n"
-        "#define ioctl(fd, func, arg) (ioctl)(fd, func, (int)(arg))\n";
+        "#define ioctl(fd, func, arg) ioctl(fd, func, (int)(arg))\n";
     c_fix_arg   = "extern[\t ]+int[\t ]+ioctl[\t ]*\\([\t ,[:alnum:]]*\\);";
         
     test_text   = "extern int ioctl ( int asdf1234, int jkl , int qwerty ) ;";