Re: [PATCH] LwIP translator
Samuel Thibault, on mer. 27 sept. 2017 00:44:15 +0200, wrote: > Last but not least, did you try to run the glibc testsuite while running > this TCP/IP stack? It would probably find potential bugs. Also, the > perl testsuite is probably a nice thing to run. (I'm not saying I won't commit this work without running those, but if running them makes you easily fix some bugs, that'll be useful :) ) Samuel
Re: [PATCH] LwIP translator
Hello,
Joan Lledó, on mer. 06 sept. 2017 10:09:36 +0200, wrote:
> --- /dev/null
> +++ b/lwip/iioctl-ops.c
> +/* Get the interface from its name */
> +static struct netif *
> +get_if (char *name)
...
> +
> + netif = netif_list;
> + while (netif != 0)
> +{
> + if (strcmp (netif_get_state (netif)->devname, ifname) == 0)
> + break;
> +
> + netif = netif->next;
> +}
Rather use a for loop:
for (netif = netif_list; netif != 0; netif = netif->next)
that's much more trustable :)
> +/* Get some sockaddr type of info. */
> +static kern_return_t
> +siocgifXaddr (struct sock_user *user,
> + ifname_t ifnam, sockaddr_t * addr, enum siocgif_type type)
> +{
> + error_t err = 0;
> + struct sockaddr_in *sin = (struct sockaddr_in *) addr;
> + struct netif *netif;
> + size_t buflen = sizeof (struct sockaddr);
> + uint32_t addrs[4];
> + err = lwip_getsockname (user->sock->sockno, addr, (socklen_t *) & buflen);
Here, perhaps comment that we are only interested in checking the
address family, thus only sizeof (struct sockaddr)
> +/* 100 SIOCGIFINDEX -- Get index number of a network interface. */
> +error_t
> +lwip_S_iioctl_siocgifindex (struct sock_user * user,
> + ifname_t ifnam,
> + int *index)
> +{
> + error_t err = 0;
> + struct netif *netif;
> + int i;
> +
> + if (!user)
> +return EOPNOTSUPP;
> +
> + i = 1; /* The first index must be 1 */
> + netif = netif_list;
> + while (netif != 0)
> +{
> + if (strcmp (netif_get_state (netif)->devname, ifnam) == 0)
> + {
> + *index = i;
> + break;
> + }
> +
> + netif = netif->next;
> + i++;
Similarly, it makes the reader more confident to read a for loop here.
> +/* 101 SIOCGIFNAME -- Get name of a network interface from index number. */
> +error_t
> +lwip_S_iioctl_siocgifname (struct sock_user * user,
> +ifname_t ifnam,
> +int *index)
> +{
And there again :)
> diff --git a/lwip/io-ops.c b/lwip/io-ops.c
> new file mode 100644
> index ..2553ca33
> --- /dev/null
> +++ b/lwip/io-ops.c
> +error_t
> +lwip_S_io_write (struct sock_user * user,
> + char *data,
> + size_t datalen,
> + off_t offset, mach_msg_type_number_t * amount)
> +{
...
> + if (sockflags & O_NONBLOCK)
> +m.msg_flags |= MSG_DONTWAIT;
> + sent = lwip_sendmsg (user->sock->sockno, &m, 0);
I'm wondering about thread-safety: I guess you enabled the unix arch to
get pthread mutex locks?
Also, why using lwip_sendmsg instead of just lwip_send? That'd avoid the
construction of m etc.
In the linuxish pfinet case, it's just because there is no recv method
:)
> +static error_t
> +lwip_io_select_common (struct sock_user *user,
> +mach_port_t reply,
> +mach_msg_type_name_t reply_type,
> +struct timeval *tv, int *select_type)
> +{
...
> + timeout = tv ? tv->tv_sec * 1000 + tv->tv_usec / 1000 : -1;
> + ret = lwip_poll (&fdp, nfds, timeout);
Better use lwip_ppoll to have better timeout resolution, and use struct
timespec instead of struct timeval.
> diff --git a/lwip/lwip-util.c b/lwip/lwip-util.c
> new file mode 100644
> index ..2996632e
> --- /dev/null
> +++ b/lwip/lwip-util.c
> + /* Freed in the module terminate callback */
> + ifc->devname = malloc (strlen (name) + 1);
> + if (ifc->devname)
> +{
> + memset (ifc->devname, 0, strlen (name) + 1);
> + strncpy (ifc->devname, name, strlen (name));
> +}
Err, rather just use strdup (name)? :)
> + if (addr6_prefix_len)
> + for (i = 0; i < LWIP_IPV6_NUM_ADDRESSES; i++)
> + *(addr6_prefix_len + i) = 64;
Does lwip support only /64 IPv6 networks?
> --- /dev/null
> +++ b/lwip/main.c
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
_GNU_SOURCE has to be defined *before* including headers. And we always
define _GNU_SOURCE while building hurd translators, this it *is* the GNU
system, so we use the GNU standard :)
> +error_t
> +trivfs_goaway (struct trivfs_control *fsys, int flags)
> +{
> + exit (0);
> +}
The linuxish pfinet returns EBUSY if there are still sockets, it is a
useful thing to check.
> + if (pi)
> +{
> + ports_port_deref (pi);
> +
> + mig_routine_t routine;
> + if ((routine = lwip_io_server_routine (inp)) ||
> + (routine = lwip_socket_server_routine (inp)) ||
> + (routine = lwip_pfinet_server_routine (inp)) ||
> + (routine = lwip_iioctl_server_routine (inp)) ||
> + (routine = NULL, trivfs_demuxer (inp, outp)))
In the linuxish pfinet, the startup protocol is also used, to nicely
close net channels before exiting, that's a useful thing to do.
> diff --git a/lwip/options.c b/lwip/options.c
> new file mode 100644
> index ..b7b34fd8
> --- /dev/null
> +++ b/lwip/options.c
> @@ -0,0 +1,346 @@
> +case 'A':
> + /* IPv6 address */
> + if
Re: BUG: /proc/self/exe reports relative paths, should always return absolute paths?
Hello, This time it looks correct :) (just a couple of nitpicks which I'll just fix). Samuel
Re: BUG: /proc/self/exe reports relative paths, should always return absolute paths?
On Tue, 2017-09-26 at 16:24 +0200, Samuel Thibault wrote:
> That should be “filename” instead of “file”.
Yes of course. That was merely a copy/paste error. Thanks for finding it.
> Take the case of PATH containing e.g. :bin: , and your current directory
> contains a bin directory which contains a script and you pass only the
> name of the script to spawni(). “file” would only contain the name
> of the script, while “filename” would contain ./bin/
Verified.
> In both cases you can just free(cwd) right after the asprintf call, to
> make the whole code simpler, and also make the cwd variable local to the
> corresponding blocks.
Fixed.2.13-33 dates when this was added
TODO: _DEBIAN_ in versions however pose problem. Remove the _DEBIAN_ version
once packages are rebuilt against 2.21.
2010-08-04 Emilio Pozuelo Monfort
* hurd/hurdexec.c (_hurd_exec): Deprecate it.
(_hurd_exec_file_name): New function.
* hurd/hurd.h (_hurd_exec): Deprecate it.
(_hurd_exec_file_name): Declare it.
* hurd/Versions: Export it.
* sysdeps/mach/hurd/execve.c: Use it.
* sysdeps/mach/hurd/fexecve.c: Likewise.
* sysdeps/mach/hurd/spawni.c: Likewise.
From d1793416cf8bf6fccd42679a8ec30b0058823ab8 Mon Sep 17 00:00:00 2001
From: Emilio Pozuelo Monfort
Date: Sat, 22 May 2010 18:26:29 +0200
Subject: [PATCH] Use the new file_exec_file_name RPC
Pass the file name of executable to the exec server, which it needs to
execute #!-scripts. Currently, the exec server tries to guess the name
from argv[0] but argv[0] only contains the executable name by convention.
---
hurd/Makefile |4 +-
hurd/Versions |4 ++
hurd/hurd.h | 14 --
hurd/hurdexec.c | 50 ++---
sysdeps/mach/hurd/execve.c |6 ++--
sysdeps/mach/hurd/fexecve.c |7 ++---
sysdeps/mach/hurd/spawni.c | 59 ++--
8 files changed, 102 insertions(+), 43 deletions(-)
Index: glibc-2.24-17.2/hurd/Versions
===
--- glibc-2.24-17.2.orig/hurd/Versions
+++ glibc-2.24-17.2/hurd/Versions
@@ -140,6 +140,14 @@ libc {
_hurd_sigstate_unlock;
_hurd_sigstate_delete;
}
+ GLIBC_2.13_DEBIAN_33 {
+# "quasi-internal" functions
+_hurd_exec_file_name;
+ }
+ GLIBC_2.21 {
+# "quasi-internal" functions
+_hurd_exec_file_name;
+ }
HURD_CTHREADS_0.3 {
# weak refs to libthreads functions that libc calls iff libthreads in use
Index: glibc-2.24-17.2/hurd/Makefile
===
--- glibc-2.24-17.2.orig/hurd/Makefile
+++ glibc-2.24-17.2/hurd/Makefile
@@ -32,8 +32,8 @@ user-interfaces := $(addprefix hurd/,\
auth auth_request auth_reply startup \
process process_request \
msg msg_reply msg_request \
- exec exec_startup crash interrupt \
- fs fsys io term tioctl socket ifsock \
+ exec exec_experimental exec_startup crash interrupt \
+ fs fs_experimental fsys io term tioctl socket ifsock \
login password pfinet \
)
server-interfaces := hurd/msg faultexc
Index: glibc-2.24-17.2/hurd/hurd.h
===
--- glibc-2.24-17.2.orig/hurd/hurd.h
+++ glibc-2.24-17.2/hurd/hurd.h
@@ -241,12 +241,20 @@ extern FILE *fopenport (io_t port, const
extern FILE *__fopenport (io_t port, const char *mode);
-/* Execute a file, replacing TASK's current program image. */
+/* Deprecated: use _hurd_exec_file_name instead. */
extern error_t _hurd_exec (task_t task,
file_t file,
char *const argv[],
- char *const envp[]);
+ char *const envp[]) __attribute_deprecated__;
+
+/* Execute a file, replacing TASK's current program image. */
+
+extern error_t _hurd_exec_file_name (task_t task,
+ file_t file,
+ const char *filename,
+ char *const argv[],
+ char *const envp[]);
/* Inform the proc server we have exited with STATUS, and kill the
Index: glibc-2.24-17.2/hurd/hurdexec.c
===
--- glibc-2.24-17.2.orig/hurd/hurdexec.c
+++ glibc-2.24-17.2/hurd/hurdexec.c
@@ -25,16 +25,37 @@
#include
#include
#include
+#include
#include
#include
+#include
+
/* Overlay TASK, executing FILE with arguments ARGV and environment ENVP.
If TASK == mach_task_self (), some ports are dealloc'd by the exec server.
- ARGV and ENVP are terminated by NULL pointers. */
+ ARGV and ENVP are terminated by NULL pointers.
+ Deprecated: use _hurd_exec_file_name instead. */
error_t
_hurd_exec (task_t task, file_t file,
char *const argv[], char *const envp[])
{
+ return _hurd_exec_file_name (task, file, NULL, argv, envp);
+}
+
+link_warning (_hurd_exec,
+ "_hurd_exec is deprecated, use _hurd_exec_file_name instead");
+
+/* Overlay TASK, executi
Re: BUG: /proc/self/exe reports relative paths, should always return absolute paths?
Svante Signell, on mar. 26 sept. 2017 16:15:06 +0200, wrote:
> + /* Relative path */
> + else
> + {
> + cwd = getcwd (NULL, 0);
> + if (cwd == NULL)
> + goto out;
> +
> + res = asprintf (&concat_name, "%s/%s", cwd, file);
That should be “filename” instead of “file”.
Take the case of PATH containing e.g. :bin: , and your current directory
contains a bin directory which contains a script and you pass only the
name of the script to spawni(). “file” would only contain the name
of the script, while “filename” would contain ./bin/
> + if (res == -1)
> + {
> + free (cwd);
In both cases you can just free(cwd) right after the asprintf call, to
make the whole code simpler, and also make the cwd variable local to the
corresponding blocks.
> + goto out;
> + }
> +
> + filename = concat_name;
> + break;
> + }
Re: BUG: /proc/self/exe reports relative paths, should always return absolute paths?
On Mon, 2017-09-25 at 18:01 +0200, Samuel Thibault wrote:
> Svante Signell, on lun. 25 sept. 2017 17:51:52 +0200, wrote:
> > The following tests still reveal unwanted behaviour: (where to change ?)
These two tests are now OK.
> It's in the PATH lookup case, just below where you added your code. It
> indeed have to cope with the case where elements in $PATH are relative
> paths (even if that's really not good practice to have e.g. "." in one's
> own $PATH)
Hopefully this version is one of the last iterations.
2.13-33 dates when this was added
TODO: _DEBIAN_ in versions however pose problem. Remove the _DEBIAN_ version
once packages are rebuilt against 2.21.
2010-08-04 Emilio Pozuelo Monfort
* hurd/hurdexec.c (_hurd_exec): Deprecate it.
(_hurd_exec_file_name): New function.
* hurd/hurd.h (_hurd_exec): Deprecate it.
(_hurd_exec_file_name): Declare it.
* hurd/Versions: Export it.
* sysdeps/mach/hurd/execve.c: Use it.
* sysdeps/mach/hurd/fexecve.c: Likewise.
* sysdeps/mach/hurd/spawni.c: Likewise.
From d1793416cf8bf6fccd42679a8ec30b0058823ab8 Mon Sep 17 00:00:00 2001
From: Emilio Pozuelo Monfort
Date: Sat, 22 May 2010 18:26:29 +0200
Subject: [PATCH] Use the new file_exec_file_name RPC
Pass the file name of executable to the exec server, which it needs to
execute #!-scripts. Currently, the exec server tries to guess the name
from argv[0] but argv[0] only contains the executable name by convention.
---
hurd/Makefile |4 +-
hurd/Versions |4 ++
hurd/hurd.h | 14 --
hurd/hurdexec.c | 50 ++---
sysdeps/mach/hurd/execve.c |6 ++--
sysdeps/mach/hurd/fexecve.c |7 ++---
sysdeps/mach/hurd/spawni.c | 59 ++--
8 files changed, 102 insertions(+), 43 deletions(-)
Index: glibc-2.24-17.2/hurd/Versions
===
--- glibc-2.24-17.2.orig/hurd/Versions
+++ glibc-2.24-17.2/hurd/Versions
@@ -140,6 +140,14 @@ libc {
_hurd_sigstate_unlock;
_hurd_sigstate_delete;
}
+ GLIBC_2.13_DEBIAN_33 {
+# "quasi-internal" functions
+_hurd_exec_file_name;
+ }
+ GLIBC_2.21 {
+# "quasi-internal" functions
+_hurd_exec_file_name;
+ }
HURD_CTHREADS_0.3 {
# weak refs to libthreads functions that libc calls iff libthreads in use
Index: glibc-2.24-17.2/hurd/Makefile
===
--- glibc-2.24-17.2.orig/hurd/Makefile
+++ glibc-2.24-17.2/hurd/Makefile
@@ -32,8 +32,8 @@ user-interfaces := $(addprefix hurd/,\
auth auth_request auth_reply startup \
process process_request \
msg msg_reply msg_request \
- exec exec_startup crash interrupt \
- fs fsys io term tioctl socket ifsock \
+ exec exec_experimental exec_startup crash interrupt \
+ fs fs_experimental fsys io term tioctl socket ifsock \
login password pfinet \
)
server-interfaces := hurd/msg faultexc
Index: glibc-2.24-17.2/hurd/hurd.h
===
--- glibc-2.24-17.2.orig/hurd/hurd.h
+++ glibc-2.24-17.2/hurd/hurd.h
@@ -241,12 +241,20 @@ extern FILE *fopenport (io_t port, const
extern FILE *__fopenport (io_t port, const char *mode);
-/* Execute a file, replacing TASK's current program image. */
+/* Deprecated: use _hurd_exec_file_name instead. */
extern error_t _hurd_exec (task_t task,
file_t file,
char *const argv[],
- char *const envp[]);
+ char *const envp[]) __attribute_deprecated__;
+
+/* Execute a file, replacing TASK's current program image. */
+
+extern error_t _hurd_exec_file_name (task_t task,
+ file_t file,
+ const char *filename,
+ char *const argv[],
+ char *const envp[]);
/* Inform the proc server we have exited with STATUS, and kill the
Index: glibc-2.24-17.2/hurd/hurdexec.c
===
--- glibc-2.24-17.2.orig/hurd/hurdexec.c
+++ glibc-2.24-17.2/hurd/hurdexec.c
@@ -25,16 +25,37 @@
#include
#include
#include
+#include
#include
#include
+#include
+
/* Overlay TASK, executing FILE with arguments ARGV and environment ENVP.
If TASK == mach_task_self (), some ports are dealloc'd by the exec server.
- ARGV and ENVP are terminated by NULL pointers. */
+ ARGV and ENVP are terminated by NULL pointers.
+ Deprecated: use _hurd_exec_file_name instead. */
error_t
_hurd_exec (task_t task, file_t file,
char *const argv[], char *const envp[])
{
+ return _hurd_exec_file_name (task, file, NULL, argv, envp);
+}
+
+link_warning (_hurd_exec,
+ "_hurd_exec is deprecated, use _hurd_exec_file_name instead");
+
+/* Overlay TASK, executing FILE with arguments ARGV and environment ENVP.
+ If TASK == mach_task_self (), some ports are dealloc'd by the exec server
