[uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Hans-Werner Hilse]

Naming local variables "stdin" and "stdout" is fine with glibc,
other C libraries, namely musl, will however complain.

Signed-off-by: Hans-Werner Hilse <hwhilse@gmail.com>
---
  arch/um/drivers/harddog_user.c         | 18 +++++++++---------
  arch/um/drivers/net_user.c             |  6 +++---
  arch/um/drivers/slip_user.c            | 14 +++++++-------
  arch/um/drivers/slirp_user.c           | 16 ++++++++--------
  arch/um/os-Linux/drivers/tuntap_user.c |  6 +++---
  5 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/arch/um/drivers/harddog_user.c 
b/arch/um/drivers/harddog_user.c
index f99b32a..3aa8b0d 100644
--- a/arch/um/drivers/harddog_user.c
+++ b/arch/um/drivers/harddog_user.c
@@ -9,8 +9,8 @@
  #include <os.h>

  struct dog_data {
-	int stdin;
-	int stdout;
+	int stdin_fd;
+	int stdout_fd;
  	int close_me[2];
  };

@@ -18,11 +18,11 @@ static void pre_exec(void *d)
  {
  	struct dog_data *data = d;

-	dup2(data->stdin, 0);
-	dup2(data->stdout, 1);
-	dup2(data->stdout, 2);
-	close(data->stdin);
-	close(data->stdout);
+	dup2(data->stdin_fd, 0);
+	dup2(data->stdout_fd, 1);
+	dup2(data->stdout_fd, 2);
+	close(data->stdin_fd);
+	close(data->stdout_fd);
  	close(data->close_me[0]);
  	close(data->close_me[1]);
  }
@@ -49,8 +49,8 @@ int start_watchdog(int *in_fd_ret, int *out_fd_ret, 
char *sock)
  		goto out_close_in;
  	}

-	data.stdin = out_fds[0];
-	data.stdout = in_fds[1];
+	data.stdin_fd = out_fds[0];
+	data.stdout_fd = in_fds[1];
  	data.close_me[0] = out_fds[1];
  	data.close_me[1] = in_fds[0];

diff --git a/arch/um/drivers/net_user.c b/arch/um/drivers/net_user.c
index cd14157..e697a41 100644
--- a/arch/um/drivers/net_user.c
+++ b/arch/um/drivers/net_user.c
@@ -166,7 +166,7 @@ int net_sendto(int fd, void *buf, int len, void *to, 
int sock_len)

  struct change_pre_exec_data {
  	int close_me;
-	int stdout;
+	int stdout_fd;
  };

  static void change_pre_exec(void *arg)
@@ -174,7 +174,7 @@ static void change_pre_exec(void *arg)
  	struct change_pre_exec_data *data = arg;

  	close(data->close_me);
-	dup2(data->stdout, 1);
+	dup2(data->stdout_fd, 1);
  }

  static int change_tramp(char **argv, char *output, int output_len)
@@ -189,7 +189,7 @@ static int change_tramp(char **argv, char *output, 
int output_len)
  		return err;
  	}
  	pe_data.close_me = fds[0];
-	pe_data.stdout = fds[1];
+	pe_data.stdout_fd = fds[1];
  	pid = run_helper(change_pre_exec, &pe_data, argv);

  	if (pid > 0)	/* Avoid hang as we won't get data in failure case. */
diff --git a/arch/um/drivers/slip_user.c b/arch/um/drivers/slip_user.c
index 55c290d..0d6b66c 100644
--- a/arch/um/drivers/slip_user.c
+++ b/arch/um/drivers/slip_user.c
@@ -55,8 +55,8 @@ static int set_up_tty(int fd)
  }

  struct slip_pre_exec_data {
-	int stdin;
-	int stdout;
+	int stdin_fd;
+	int stdout_fd;
  	int close_me;
  };

@@ -64,9 +64,9 @@ static void slip_pre_exec(void *arg)
  {
  	struct slip_pre_exec_data *data = arg;

-	if (data->stdin >= 0)
-		dup2(data->stdin, 0);
-	dup2(data->stdout, 1);
+	if (data->stdin_fd >= 0)
+		dup2(data->stdin_fd, 0);
+	dup2(data->stdout_fd, 1);
  	if (data->close_me >= 0)
  		close(data->close_me);
  }
@@ -85,8 +85,8 @@ static int slip_tramp(char **argv, int fd)
  	}

  	err = 0;
-	pe_data.stdin = fd;
-	pe_data.stdout = fds[1];
+	pe_data.stdin_fd = fd;
+	pe_data.stdout_fd = fds[1];
  	pe_data.close_me = fds[0];
  	err = run_helper(slip_pre_exec, &pe_data, argv);
  	if (err < 0)
diff --git a/arch/um/drivers/slirp_user.c b/arch/um/drivers/slirp_user.c
index c999d18..98b6a41 100644
--- a/arch/um/drivers/slirp_user.c
+++ b/arch/um/drivers/slirp_user.c
@@ -20,18 +20,18 @@ static int slirp_user_init(void *data, void *dev)
  }

  struct slirp_pre_exec_data {
-	int stdin;
-	int stdout;
+	int stdin_fd;
+	int stdout_fd;
  };

  static void slirp_pre_exec(void *arg)
  {
  	struct slirp_pre_exec_data *data = arg;

-	if (data->stdin != -1)
-		dup2(data->stdin, 0);
-	if (data->stdout != -1)
-		dup2(data->stdout, 1);
+	if (data->stdin_fd != -1)
+		dup2(data->stdin_fd, 0);
+	if (data->stdout_fd != -1)
+		dup2(data->stdout_fd, 1);
  }

  static int slirp_tramp(char **argv, int fd)
@@ -39,8 +39,8 @@ static int slirp_tramp(char **argv, int fd)
  	struct slirp_pre_exec_data pe_data;
  	int pid;

-	pe_data.stdin = fd;
-	pe_data.stdout = fd;
+	pe_data.stdin_fd = fd;
+	pe_data.stdout_fd = fd;
  	pid = run_helper(slirp_pre_exec, &pe_data, argv);

  	return pid;
diff --git a/arch/um/os-Linux/drivers/tuntap_user.c 
b/arch/um/os-Linux/drivers/tuntap_user.c
index 14126d9..c2e6e1d 100644
--- a/arch/um/os-Linux/drivers/tuntap_user.c
+++ b/arch/um/os-Linux/drivers/tuntap_user.c
@@ -47,7 +47,7 @@ static void tuntap_del_addr(unsigned char *addr, 
unsigned char *netmask,
  }

  struct tuntap_pre_exec_data {
-	int stdout;
+	int stdout_fd;
  	int close_me;
  };

@@ -55,7 +55,7 @@ static void tuntap_pre_exec(void *arg)
  {
  	struct tuntap_pre_exec_data *data = arg;

-	dup2(data->stdout, 1);
+	dup2(data->stdout_fd, 1);
  	close(data->close_me);
  }

@@ -74,7 +74,7 @@ static int tuntap_open_tramp(char *gate, int *fd_out, 
int me, int remote,

  	sprintf(version_buf, "%d", UML_NET_VERSION);

-	data.stdout = remote;
+	data.stdout_fd = remote;
  	data.close_me = me;

  	pid = run_helper(tuntap_pre_exec, &data, argv);
-- 
2.4.2



------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Richard Weinberger]

On Wed, Jun 10, 2015 at 11:21 PM, Hans-Werner Hilse <hwhilse@gmail.com> wrote:
> Naming local variables "stdin" and "stdout" is fine with glibc,
> other C libraries, namely musl, will however complain.

Wait, what?!
These are not local variables, they are struct members.

What exactly is the issue with musl? Sounds very odd.

-- 
Thanks,
//richard

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Hans-Werner Hilse]

Hi,

Am 2015-06-10 23:37, schrieb Richard Weinberger:
> On Wed, Jun 10, 2015 at 11:21 PM, Hans-Werner Hilse <hwhilse@gmail.com> 
> wrote:
>> Naming local variables "stdin" and "stdout" is fine with glibc,
>> other C libraries, namely musl, will however complain.
> 
> Wait, what?!
> These are not local variables, they are struct members.
> 
> What exactly is the issue with musl? Sounds very odd.

They chose to have stdin and stdout (and, of course, stderr) wrapped in 
macros - in a different way than glibc:

[...]
   extern FILE *const stdin;
[...]
   #define stdin  (stdin)

(http://git.musl-libc.org/cgit/musl/tree/include/stdio.h?id=v1.1.10#n59)

The C standards C89/C99 (the last one I even looked up right now) say 
those identifiers are macros - according to stdio.h from glibc, which 
does the following:

   /* Standard streams.  */
   extern struct _IO_FILE *stdin;          /* Standard input stream.  */
[...]
   /* C89/C99 say they're macros.  Make them happy.  */
   #define stdin stdin
[...]

Of course you're right, of course they are struct members. That's what I 
get for writing my commit messages too long after going at the issue at 
hand.

I'll send an updated patch series in a few minutes.

Thanks for getting back and caring!

-hwh

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Richard Weinberger]

On Thu, Jun 11, 2015 at 8:35 AM, Hans-Werner Hilse <hwhilse@gmail.com> wrote:
> Hi,
>
> Am 2015-06-10 23:37, schrieb Richard Weinberger:
>> On Wed, Jun 10, 2015 at 11:21 PM, Hans-Werner Hilse <hwhilse@gmail.com>
>> wrote:
>>> Naming local variables "stdin" and "stdout" is fine with glibc,
>>> other C libraries, namely musl, will however complain.
>>
>> Wait, what?!
>> These are not local variables, they are struct members.
>>
>> What exactly is the issue with musl? Sounds very odd.
>
> They chose to have stdin and stdout (and, of course, stderr) wrapped in
> macros - in a different way than glibc:
>
> [...]
>    extern FILE *const stdin;
> [...]
>    #define stdin  (stdin)
>
> (http://git.musl-libc.org/cgit/musl/tree/include/stdio.h?id=v1.1.10#n59)
>
> The C standards C89/C99 (the last one I even looked up right now) say
> those identifiers are macros - according to stdio.h from glibc, which
> does the following:
>
>    /* Standard streams.  */
>    extern struct _IO_FILE *stdin;          /* Standard input stream.  */
> [...]
>    /* C89/C99 say they're macros.  Make them happy.  */
>    #define stdin stdin
> [...]
>
> Of course you're right, of course they are struct members. That's what I
> get for writing my commit messages too long after going at the issue at
> hand.
>
> I'll send an updated patch series in a few minutes.

Slow down a bit. Since when is it forbidden to name something "stdin" in a C
program? Does the standard say that? If yes, where?
We have to sort this out. If UML violates a rule, we have to fix it.
But maybe musl needs fixing...

-- 
Thanks,
//richard

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Hans-Werner Hilse]

Am 2015-06-11 08:49, schrieb Richard Weinberger:
> On Thu, Jun 11, 2015 at 8:35 AM, Hans-Werner Hilse <hwhilse@gmail.com> 
> wrote:
>> Hi,
>> 
>> Am 2015-06-10 23:37, schrieb Richard Weinberger:
>>> On Wed, Jun 10, 2015 at 11:21 PM, Hans-Werner Hilse 
>>> <hwhilse@gmail.com>
>>> wrote:
>>>> Naming local variables "stdin" and "stdout" is fine with glibc,
>>>> other C libraries, namely musl, will however complain.
>>> 
>>> Wait, what?!
>>> These are not local variables, they are struct members.
>>> 
>>> What exactly is the issue with musl? Sounds very odd.
>> 
>> They chose to have stdin and stdout (and, of course, stderr) wrapped 
>> in
>> macros - in a different way than glibc:
>> 
>> [...]
> 
> Slow down a bit. Since when is it forbidden to name something "stdin" 
> in a C
> program? Does the standard say that? If yes, where?
> We have to sort this out. If UML violates a rule, we have to fix it.
> But maybe musl needs fixing...

Digging further, I think I cannot better describe the issue than Rich 
Felker did in this message:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130506/173690.html

> Per ISO C, stdin, stdout, and stderr are macros "which are expressions
> of type ''pointer to FILE'' that point to the FILE objects associated,
> respectively, with the standard error, input, and output streams."
> (C99 7.19.1 paragraph 3). They are not necessarily objects, merely
> expressions, and as such, taking their address is invalid.

That last sentence, however, does not apply to the problem I'm facing 
with UML and musl. Here, the problem is actually that the 
stdin/stdout/stderr identifiers are per the C standard predefined as 
being macros.

In the glibc case, it won't break anyway since the macro definition is 
tight enough to mask the problem. For musl, the macro definition is 
minimally more complex and the result is a macro expansion that leaves 
syntactically incorrect code. In the past - see the linked discussion - 
musl developers stressed that their implementation is conformant to the 
standard.

About forbidden use of names/identifiers from the stdlib, the C standard 
is a bit hard for me to grasp fully. However, I think C99 7.1.3 
paragraph 1, 3rd point is relevant here: "Each macro name in any of the 
following subclauses (including the future library directions) is 
reserved for use as specified if any of its associated headers is 
included; unless explicitly stated otherwise (see 7.1.4)."

So aside from renaming struct members, another option would be to lift 
the macro definition by an "#undef" in relevant places. I have a 
feeling, though, that this faces new problems with the C99 standard, 
7.1.3 paragraph 3, which says that "If the program removes (with #undef) 
any macro definition of an identifier in the first group listed above, 
the behavior is undefined."

-hwh


------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Richard Weinberger]

On Thu, Jun 11, 2015 at 9:54 AM, Hans-Werner Hilse <hwhilse@gmail.com> wrote:
> Am 2015-06-11 08:49, schrieb Richard Weinberger:
>> On Thu, Jun 11, 2015 at 8:35 AM, Hans-Werner Hilse <hwhilse@gmail.com>
>> wrote:
>>> Hi,
>>>
>>> Am 2015-06-10 23:37, schrieb Richard Weinberger:
>>>> On Wed, Jun 10, 2015 at 11:21 PM, Hans-Werner Hilse
>>>> <hwhilse@gmail.com>
>>>> wrote:
>>>>> Naming local variables "stdin" and "stdout" is fine with glibc,
>>>>> other C libraries, namely musl, will however complain.
>>>>
>>>> Wait, what?!
>>>> These are not local variables, they are struct members.
>>>>
>>>> What exactly is the issue with musl? Sounds very odd.
>>>
>>> They chose to have stdin and stdout (and, of course, stderr) wrapped
>>> in
>>> macros - in a different way than glibc:
>>>
>>> [...]
>>
>> Slow down a bit. Since when is it forbidden to name something "stdin"
>> in a C
>> program? Does the standard say that? If yes, where?
>> We have to sort this out. If UML violates a rule, we have to fix it.
>> But maybe musl needs fixing...
>
> Digging further, I think I cannot better describe the issue than Rich
> Felker did in this message:
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130506/173690.html
>
>> Per ISO C, stdin, stdout, and stderr are macros "which are expressions
>> of type ''pointer to FILE'' that point to the FILE objects associated,
>> respectively, with the standard error, input, and output streams."
>> (C99 7.19.1 paragraph 3). They are not necessarily objects, merely
>> expressions, and as such, taking their address is invalid.
>
> That last sentence, however, does not apply to the problem I'm facing
> with UML and musl. Here, the problem is actually that the
> stdin/stdout/stderr identifiers are per the C standard predefined as
> being macros.
>
> In the glibc case, it won't break anyway since the macro definition is
> tight enough to mask the problem. For musl, the macro definition is
> minimally more complex and the result is a macro expansion that leaves
> syntactically incorrect code. In the past - see the linked discussion -
> musl developers stressed that their implementation is conformant to the
> standard.
>
> About forbidden use of names/identifiers from the stdlib, the C standard
> is a bit hard for me to grasp fully. However, I think C99 7.1.3
> paragraph 1, 3rd point is relevant here: "Each macro name in any of the
> following subclauses (including the future library directions) is
> reserved for use as specified if any of its associated headers is
> included; unless explicitly stated otherwise (see 7.1.4)."

Yeah, but we're talking about structure members.

> So aside from renaming struct members, another option would be to lift
> the macro definition by an "#undef" in relevant places. I have a
> feeling, though, that this faces new problems with the C99 standard,
> 7.1.3 paragraph 3, which says that "If the program removes (with #undef)
> any macro definition of an identifier in the first group listed above,
> the behavior is undefined."

So, what exactly is the build error you're facing and how can I reproduce?

-- 
Thanks,
//richard

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Hans-Werner Hilse]

Am 2015-06-11 10:09, schrieb Richard Weinberger:
> On Thu, Jun 11, 2015 at 9:54 AM, Hans-Werner Hilse <hwhilse@gmail.com> 
> wrote:
>> About forbidden use of names/identifiers from the stdlib, the C 
>> standard
>> is a bit hard for me to grasp fully. However, I think C99 7.1.3
>> paragraph 1, 3rd point is relevant here: "Each macro name in any of 
>> the
>> following subclauses (including the future library directions) is
>> reserved for use as specified if any of its associated headers is
>> included; unless explicitly stated otherwise (see 7.1.4)."
> 
> Yeah, but we're talking about structure members.

Indeed, and I can't find more detail about the "reserved for use as 
specified" part.
I think it is saying, though, that those identifiers are macros (and as 
such will get expanded). The preprocessor will not care if that's in a 
struct member name.

>> So aside from renaming struct members, another option would be to lift
>> the macro definition by an "#undef" in relevant places. I have a
>> feeling, though, that this faces new problems with the C99 standard,
>> 7.1.3 paragraph 3, which says that "If the program removes (with 
>> #undef)
>> any macro definition of an identifier in the first group listed above,
>> the behavior is undefined."
> 
> So, what exactly is the build error you're facing and how can I 
> reproduce?

Of course I should have told you in the first place:

[hw@hilses linux-uml]$ make ARCH=um CC="musl-gcc"
...
   CC      arch/um/drivers/slip_user.o
In file included from arch/um/drivers/slip_user.c:12:0:
/usr/lib/musl/include/sys/termios.h:1:2: warning: #warning redirecting 
incorrect #include <sys/termios.h> to <termios.h> [-Wcpp]
  #warning redirecting incorrect #include <sys/termios.h> to <termios.h>
   ^
In file included from arch/um/drivers/slip_user.c:6:0:
arch/um/drivers/slip_user.c: In function ‘slip_pre_exec’:
arch/um/drivers/slip_user.c:67:12: error: expected identifier before ‘(’ 
token
   if (data->stdin >= 0)
             ^
arch/um/drivers/slip_user.c:68:14: error: expected identifier before ‘(’ 
token
    dup2(data->stdin, 0);
               ^
arch/um/drivers/slip_user.c:68:3: error: too few arguments to function 
‘dup2’
    dup2(data->stdin, 0);
    ^

(We can ignore the warning for now/this discussion, I think.)

You can reproduce without actually installing a musl toolchain when 
changing glibc's macro definition to musl's:

make ARCH=um CFLAGS="-Dstdout=\\(stdout\\)"

Will trigger the same error in the same place. Of course, this is 
artificially producing it. Using musl, however, leads to the same 
behaviour.

I used the defconfig, that's why it triggers in slip_user.c first - 
other places do use stdin/stdout as struct member name, too.

-hwh

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [PATCH 2/3] um: Do not use stdin and stdout names for variables

[Richard Weinberger]

On Thu, Jun 11, 2015 at 10:26 AM, Hans-Werner Hilse <hwhilse@gmail.com> wrote:
> Am 2015-06-11 10:09, schrieb Richard Weinberger:
>> On Thu, Jun 11, 2015 at 9:54 AM, Hans-Werner Hilse <hwhilse@gmail.com>
>> wrote:
>>> About forbidden use of names/identifiers from the stdlib, the C
>>> standard
>>> is a bit hard for me to grasp fully. However, I think C99 7.1.3
>>> paragraph 1, 3rd point is relevant here: "Each macro name in any of
>>> the
>>> following subclauses (including the future library directions) is
>>> reserved for use as specified if any of its associated headers is
>>> included; unless explicitly stated otherwise (see 7.1.4)."
>>
>> Yeah, but we're talking about structure members.
>
> Indeed, and I can't find more detail about the "reserved for use as
> specified" part.
> I think it is saying, though, that those identifiers are macros (and as
> such will get expanded). The preprocessor will not care if that's in a
> struct member name.

While I think musl could be more user friendly,
I fear the easiest solution is really to rename our structure members.
glibc does a the user a favor by having a simple "#define stdin stdin".

>>> So aside from renaming struct members, another option would be to lift
>>> the macro definition by an "#undef" in relevant places. I have a
>>> feeling, though, that this faces new problems with the C99 standard,
>>> 7.1.3 paragraph 3, which says that "If the program removes (with
>>> #undef)
>>> any macro definition of an identifier in the first group listed above,
>>> the behavior is undefined."
>>
>> So, what exactly is the build error you're facing and how can I
>> reproduce?
>
> Of course I should have told you in the first place:
>
> [hw@hilses linux-uml]$ make ARCH=um CC="musl-gcc"

Thx!

-- 
Thanks,
//richard

------------------------------------------------------------------------------
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel