[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: exec server and /dev/fd/N
|
From: |
Carl Fredrik Hammar |
|
Subject: |
Re: exec server and /dev/fd/N |
|
Date: |
Tue, 25 May 2010 21:10:57 +0200 |
|
User-agent: |
Mutt/1.5.20 (2009-06-14) |
Hi,
On Mon, May 24, 2010 at 12:08:10PM +0200, Emilio Pozuelo Monfort wrote:
>
> These are a series of patches to fix https://savannah.gnu.org/bugs/?28934
>
> Basically the problem is that in some cases the exec server can't find the
> file
> name of the file being executed (when it's a script), and then makes a hack
> passing /dev/fd/N to the interpreter.
>
> I tried to fix it with heuristics such as "if the file name contains no
> slashes,
> then it's relative" and passing just a flag to the exec server to avoid
> needing
> to create new RPCs, but that's not enough, since a call like
> execv("foo",{"bar",
> NULL}) (i.e. filename != argv[0]) should work too. So the only option is to
> pass
> filename to the exec server, and just use that.
>
> So this patch adds two new RPCs: exec_exec_file_name RPC and
> file_exec_file_name
> one. Then libc can use exec_exec_file_name in hurdexec.c.
Good job! There's still work to do but it's a pretty solid base.
First up, a couple of overall suggestions.
Some of the changes that need to be made have already been discussed on
IRC: you should use string_t instead of data_t, empty string instead of
NULL for RPCs, and update copyright years,
The first three patches are pretty useless on their own, they all affect
the same program, and most changes are pretty mechanical, so I think
you might as well merge them.
You also haven't changed some calls to exec_exec and _hurd_exec() in
Hurd and glib to _hurd_exec_file_name(), which would be appropriate if
we're deprecating _hurd_exec().
It would also be good if you always include a ChangeLog so I can catch
early errors there too, which hopefully leads to less round-trips.
Are you familiar with TopGit? It is pretty useful when developing
patch series since you can keep history of the development of a patch.
It is also nice since it helps you maintain the ChangeLog if you keep
it in .topmsg, which will later become the commit message for the patch.
Now to the details. I've commented on every hunk to make sure I looked
through it all, which makes it a bit long but hopefully easy to follow
(complain if it isn't).
> --- a/exec/exec.c
> +++ b/exec/exec.c
> @@ -1433,7 +1433,7 @@ do_exec (file_t file,
> {
> /* Check for a #! executable file. */
> check_hashbang (&e,
> - file, oldtask, flags,
> + file, oldtask, flags, NULL,
> argv, argvlen, argv_copy,
> envp, envplen, envp_copy,
> dtable, dtablesize, dtable_copy,
Not needed if merged with patch 2.
> diff --git a/exec/hashexec.c b/exec/hashexec.c
> index 2aa3844..7a7b329 100644
> --- a/exec/hashexec.c
> +++ b/exec/hashexec.c
> @@ -35,6 +35,7 @@ check_hashbang (struct execdata *e,
> file_t file,
> task_t oldtask,
> int flags,
> + char *filename,
> char *argv, u_int argvlen, boolean_t argv_copy,
> char *envp, u_int envplen, boolean_t envp_copy,
> mach_port_t *dtable, u_int dtablesize, boolean_t dtable_copy,
Good.
> @@ -225,7 +226,8 @@ check_hashbang (struct execdata *e,
> file_name = NULL;
> else if (! (flags & EXEC_SECURE))
> {
> - /* Try to figure out the file's name. We guess that if ARGV[0]
> + /* Try to figure out the file's name. If FILENAME is not NULL,
> + then it's the file's name. Otherwise we guess that if ARGV[0]
> contains a slash, it might be the name of the file; and that
> if it contains no slash, looking for files named by ARGV[0] in
> the `PATH' environment variable might find it. */
There should be two spaces after end of a sentence.
> @@ -278,7 +280,9 @@ check_hashbang (struct execdata *e,
> else
> name = argv;
>
> - if (strchr (name, '/') != NULL)
> + if (filename)
> + error = lookup (name = filename, 0, &name_file);
> + else if (strchr (name, '/') != NULL)
> error = lookup (name, 0, &name_file);
> else if ((error = hurd_catch_signal
> (sigmask (SIGBUS) | sigmask (SIGSEGV),
Should check for "" instead of null.
> diff --git a/exec/priv.h b/exec/priv.h
> index 7cee15e..fa41f7c 100644
> --- a/exec/priv.h
> +++ b/exec/priv.h
> @@ -171,6 +171,7 @@ void check_hashbang (struct execdata *e,
> file_t file,
> task_t oldtask,
> int flags,
> + char *filename,
> char *argv, u_int argvlen, boolean_t argv_copy,
> char *envp, u_int envplen, boolean_t envp_copy,
> mach_port_t *dtable, u_int dtablesize,
Good.
> >From 80d9b08fc69f694448d5d53dea16cf14e54d1dde Mon Sep 17 00:00:00 2001
> From: Emilio Pozuelo Monfort <pochu27@gmail.com>
> Date: Wed, 19 May 2010 23:34:12 +0200
> Subject: [PATCH 2/4] Add a filename argument to do_exec
>
> ---
> exec/exec.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/exec/exec.c b/exec/exec.c
> index 1ea4d92..ea2bd5d 100644
> --- a/exec/exec.c
> +++ b/exec/exec.c
> @@ -1338,6 +1338,7 @@ static error_t
> do_exec (file_t file,
> task_t oldtask,
> int flags,
> + char *filename,
> char *argv, mach_msg_type_number_t argvlen, boolean_t argv_copy,
> char *envp, mach_msg_type_number_t envplen, boolean_t envp_copy,
> mach_port_t *dtable, mach_msg_type_number_t dtablesize,
Good.
> @@ -1433,7 +1434,7 @@ do_exec (file_t file,
> {
> /* Check for a #! executable file. */
> check_hashbang (&e,
> - file, oldtask, flags, NULL,
> + file, oldtask, flags, filename,
> argv, argvlen, argv_copy,
> envp, envplen, envp_copy,
> dtable, dtablesize, dtable_copy,
Good.
> @@ -2092,7 +2093,7 @@ S_exec_exec (struct trivfs_protid *protid,
> trivfs_protid_portclasses[0]);
> if (protid)
> {
> - err = do_exec (file, oldtask, 0,
> + err = do_exec (file, oldtask, 0, NULL,
> argv, argvlen, argv_copy,
> envp, envplen, envp_copy,
> dtable, dtablesize, dtable_copy,
Not needed if merged with patch 3.
> @@ -2139,7 +2140,7 @@ S_exec_exec (struct trivfs_protid *protid,
> /* There were no user-specified exec servers,
> or none of them could be found. */
>
> - return do_exec (file, oldtask, flags,
> + return do_exec (file, oldtask, flags, NULL,
> argv, argvlen, argv_copy,
> envp, envplen, envp_copy,
> dtable, dtablesize, dtable_copy,
Likewise.
> >From 4d42ddb105b4560a3bcefb875fb7961e526d9876 Mon Sep 17 00:00:00 2001
> From: Emilio Pozuelo Monfort <pochu27@gmail.com>
> Date: Thu, 20 May 2010 23:53:08 +0200
> Subject: [PATCH 3/4] Add a new exec_exec_file_name RPC
>
> It's similar to exec_exec but with an additional filename
> argument. This deprecates exec_exec.
> ---
> exec/exec.c | 39 +++++++++++++++++++++++++++++++++++++--
> hurd/exec.defs | 16 +++++++++++++++-
> 2 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/exec/exec.c b/exec/exec.c
> index ea2bd5d..7b51ff6 100644
> --- a/exec/exec.c
> +++ b/exec/exec.c
> @@ -2037,6 +2037,7 @@ do_exec (file_t file,
> return e.error;
> }
>
> +/* Deprecated. */
> kern_return_t
> S_exec_exec (struct trivfs_protid *protid,
> file_t file,
Good.
> @@ -2053,6 +2054,40 @@ S_exec_exec (struct trivfs_protid *protid,
> mach_port_t *deallocnames, mach_msg_type_number_t ndeallocnames,
> mach_port_t *destroynames, mach_msg_type_number_t ndestroynames)
> {
> + return S_exec_exec_file_name (protid,
> + file,
> + oldtask,
> + flags,
> + NULL, 0,
> + argv, argvlen, argv_copy,
> + envp, envplen, envp_copy,
> + dtable, dtablesize,
> + dtable_copy,
> + portarray, nports,
> + portarray_copy,
> + intarray, nints,
> + intarray_copy,
> + deallocnames, ndeallocnames,
> + destroynames, ndestroynames);
> +}
> +
>
> +kern_return_t
> +S_exec_exec_file_name (struct trivfs_protid *protid,
> + file_t file,
> + task_t oldtask,
> + int flags,
> + char *filename, mach_msg_type_number_t filenamelen,
> + char *argv, mach_msg_type_number_t argvlen, boolean_t
> argv_copy,
> + char *envp, mach_msg_type_number_t envplen, boolean_t
> envp_copy,
> + mach_port_t *dtable, mach_msg_type_number_t dtablesize,
> + boolean_t dtable_copy,
> + mach_port_t *portarray, mach_msg_type_number_t nports,
> + boolean_t portarray_copy,
> + int *intarray, mach_msg_type_number_t nints,
> + boolean_t intarray_copy,
> + mach_port_t *deallocnames, mach_msg_type_number_t
> ndeallocnames,
> + mach_port_t *destroynames, mach_msg_type_number_t
> ndestroynames)
> +{
> if (! protid)
> return EOPNOTSUPP;
Drop filenamelen and use "" instead of NULL.
> @@ -2093,7 +2128,7 @@ S_exec_exec (struct trivfs_protid *protid,
> trivfs_protid_portclasses[0]);
> if (protid)
> {
> - err = do_exec (file, oldtask, 0, NULL,
> + err = do_exec (file, oldtask, 0, filename,
> argv, argvlen, argv_copy,
> envp, envplen, envp_copy,
> dtable, dtablesize, dtable_copy,
Good.
> @@ -2140,7 +2175,7 @@ S_exec_exec (struct trivfs_protid *protid,
> /* There were no user-specified exec servers,
> or none of them could be found. */
>
> - return do_exec (file, oldtask, flags, NULL,
> + return do_exec (file, oldtask, flags, filename,
> argv, argvlen, argv_copy,
> envp, envplen, envp_copy,
> dtable, dtablesize, dtable_copy,
Good.
> diff --git a/hurd/exec.defs b/hurd/exec.defs
> index 2888fb1..ad558a2 100644
> --- a/hurd/exec.defs
> +++ b/hurd/exec.defs
> @@ -29,6 +29,7 @@ EXEC_IMPORTS
>
> INTR_INTERFACE
>
> +/* Deprecated. Use exec_exec_file_name instead. */
> routine exec_exec (
> execserver: file_t;
> file: mach_port_send_t;
Only one space at start of comment and two after a sentence.
A colon would be better: ``Deprecated: use ...''.
> @@ -42,7 +43,20 @@ routine exec_exec (
> deallocnames: mach_port_name_array_t;
> destroynames: mach_port_name_array_t);
>
> -skip; /* obsolete exec_startup */
> +routine exec_exec_file_name (
> + execserver: file_t;
> + file: mach_port_send_t;
> + oldtask: task_t;
> + flags: int;
> + filename: data_t;
> + argv: data_t SCP;
> + envp: data_t SCP;
> + dtable: portarray_t SCP;
> + portarray: portarray_t SCP;
> + intarray: intarray_t SCP;
> + deallocnames: mach_port_name_array_t;
> + destroynames: mach_port_name_array_t);
> +
>
> /* This call is made by the bootstrapping filesystem to give the
> execserver its auth handle. */
filename should be string_t.
We discussed whether reusing an obsolete RPC number is a good idea on IRC.
It is probably OK in this case since it groups the new RPC with the old one,
and the skipped exec_startup was removed back in '95.
> >From d322e644ffb231de7ac37f4951fc9bb091b1e6d7 Mon Sep 17 00:00:00 2001
> From: Emilio Pozuelo Monfort <pochu27@gmail.com>
> Date: Sat, 22 May 2010 17:46:56 +0200
> Subject: [PATCH 4/4] Add a file_exec_file_name RPC
>
> Make file_exec call file_exec_file_name and deprecate file_exec.
> ---
> hurd/fs.defs | 21 ++++++++++++++++++-
> libdiskfs/file-exec.c | 52 +++++++++++++++++++++++++++++++++++++++++-------
> libnetfs/file-exec.c | 51 ++++++++++++++++++++++++++++++++++++++++-------
> libtreefs/s-file.c | 22 ++++++++++++++++++++
> libtrivfs/file-exec.c | 26 ++++++++++++++++++++++++
> trans/fakeroot.c | 48 ++++++++++++++++++++++++++++++++++++++++----
> 6 files changed, 198 insertions(+), 22 deletions(-)
>
> diff --git a/hurd/fs.defs b/hurd/fs.defs
> index 52d83bd..6f6202e 100644
> --- a/hurd/fs.defs
> +++ b/hurd/fs.defs
> @@ -35,7 +35,8 @@ INTR_INTERFACE
> /* Overlay a task with a file. Necessary initialization, including
> authentication changes associated with set[ug]id execution must be
> handled by the filesystem. Filesystems normally implement this by
> - using exec_newtask or exec_loadtask as appropriate. */
> + using exec_newtask or exec_loadtask as appropriate.
> + Deprecated. Use file_exec_file_name instead. */
Use colon instead of period.
> routine file_exec (
> exec_file: file_t;
> RPT
> @@ -352,3 +353,21 @@ routine file_reparent (
> RPT
> parent: mach_port_t;
> out new_file: mach_port_send_t);
> +
> +/* Overlay a task with a file. Necessary initialization, including
> + authentication changes associated with set[ug]id execution must be
> + handled by the filesystem. Filesystems normally implement this by
> + using exec_newtask or exec_loadtask as appropriate. */
> +routine file_exec_file_name (
> + exec_file: file_t;
> + RPT
> + exec_task: task_t;
> + flags: int;
> + filename: data_t;
> + argv: data_t SCP;
> + envp: data_t SCP;
> + fdarray: portarray_t SCP;
> + portarray: portarray_t SCP;
> + intarray: intarray_t SCP;
> + deallocnames: mach_port_name_array_t SCP;
> + destroynames: mach_port_name_array_t SCP);
filename should be string_t.
> diff --git a/libdiskfs/file-exec.c b/libdiskfs/file-exec.c
> index 452240c..511a8c2 100644
> --- a/libdiskfs/file-exec.c
> +++ b/libdiskfs/file-exec.c
> @@ -47,6 +47,41 @@ diskfs_S_file_exec (struct protid *cred,
> mach_port_t *destroynames,
> size_t destroynameslen)
> {
> + return diskfs_S_file_exec_file_name (
> + cred,
> + task,
> + flags,
> + NULL, 0,
> + argv, argvlen,
> + envp, envplen,
> + fds, fdslen,
> + portarray, portarraylen,
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> +}
> +
> +kern_return_t
> +diskfs_S_file_exec_file_name (struct protid *cred,
> + task_t task,
> + int flags,
> + char *filename,
> + size_t filenamelen,
> + char *argv,
> + size_t argvlen,
> + char *envp,
> + size_t envplen,
> + mach_port_t *fds,
> + size_t fdslen,
> + mach_port_t *portarray,
> + size_t portarraylen,
> + int *intarray,
> + size_t intarraylen,
> + mach_port_t *deallocnames,
> + size_t deallocnameslen,
> + mach_port_t *destroynames,
> + size_t destroynameslen)
> +{
> struct node *np;
> uid_t uid;
> gid_t gid;
No filenamelen and "" instead of NULL.
> @@ -159,14 +194,15 @@ diskfs_S_file_exec (struct protid *cred,
> do
> {
> right = ports_get_send_right (newpi);
> - err = exec_exec (execserver,
> - right, MACH_MSG_TYPE_COPY_SEND,
> - task, flags, argv, argvlen, envp, envplen,
> - fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> - portarray, MACH_MSG_TYPE_COPY_SEND, portarraylen,
> - intarray, intarraylen,
> - deallocnames, deallocnameslen,
> - destroynames, destroynameslen);
> + err = exec_exec_file_name (execserver,
> + right, MACH_MSG_TYPE_COPY_SEND,
> + task, flags, filename, filenamelen,
> + argv, argvlen, envp, envplen,
> + fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> + portarray, MACH_MSG_TYPE_COPY_SEND,
> portarraylen,
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> mach_port_deallocate (mach_task_self (), right);
> if (err == MACH_SEND_INVALID_DEST)
> {
No filenamelen.
> diff --git a/libnetfs/file-exec.c b/libnetfs/file-exec.c
> index 73c125b..364344b 100644
> --- a/libnetfs/file-exec.c
> +++ b/libnetfs/file-exec.c
> @@ -49,6 +49,40 @@ netfs_S_file_exec (struct protid *cred,
> mach_port_t *destroynames,
> size_t destroynameslen)
> {
> + return netfs_S_file_exec_file_name (cred,
> + task,
> + flags,
> + NULL, 0,
> + argv, argvlen,
> + envp, envplen,
> + fds, fdslen,
> + portarray, portarraylen,
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> +}
> +
> +kern_return_t
> +netfs_S_file_exec_file_name (struct protid *cred,
> + task_t task,
> + int flags,
> + char *filename,
> + size_t filenamelen,
> + char *argv,
> + size_t argvlen,
> + char *envp,
> + size_t envplen,
> + mach_port_t *fds,
> + size_t fdslen,
> + mach_port_t *portarray,
> + size_t portarraylen,
> + int *intarray,
> + size_t intarraylen,
> + mach_port_t *deallocnames,
> + size_t deallocnameslen,
> + mach_port_t *destroynames,
> + size_t destroynameslen)
> +{
No filenamelen and "" instead of NULL.
> struct node *np;
> error_t err;
> uid_t uid;
> @@ -133,14 +167,15 @@ netfs_S_file_exec (struct protid *cred,
> if (newpi)
> {
> right = ports_get_send_right (newpi);
> - err = exec_exec (_netfs_exec,
> - right, MACH_MSG_TYPE_COPY_SEND,
> - task, flags, argv, argvlen, envp, envplen,
> - fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> - portarray, MACH_MSG_TYPE_COPY_SEND, portarraylen,
> - intarray, intarraylen,
> - deallocnames, deallocnameslen,
> - destroynames, destroynameslen);
> + err = exec_exec_file_name (_netfs_exec,
> + right, MACH_MSG_TYPE_COPY_SEND,
> + task, flags, filename, filenamelen,
> + argv, argvlen, envp, envplen,
> + fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> + portarray, MACH_MSG_TYPE_COPY_SEND,
> portarraylen,
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> mach_port_deallocate (mach_task_self (), right);
> ports_port_deref (newpi);
> }
No filenamelen.
> diff --git a/libtreefs/s-file.c b/libtreefs/s-file.c
> index 73c32d7..2f6cedd 100644
> --- a/libtreefs/s-file.c
> +++ b/libtreefs/s-file.c
> @@ -93,6 +93,28 @@ treefs_S_file_exec (struct treefs_protid *cred,
> }
>
> error_t
> +treefs_S_file_exec_file_name (struct treefs_protid *cred,
> + task_t task, int flags,
> + char *filename, unsigned filename_len,
> + char *argv, unsigned argv_len,
> + char *envp, unsigned envp_len,
> + mach_port_t *fds, unsigned fds_len,
> + mach_port_t *ports, unsigned ports_len,
> + int *ints, unsigned ints_len,
> + mach_port_t *dealloc, unsigned dealloc_len,
> + mach_port_t *destroy, unsigned destroy_len)
> +{
> + if (!cred)
> + return EOPNOTSUPP;
> + /* XXX We're ignoring filename, so this is the same as if the caller would
> + have called file_exec instead of file_exec_file_name. */
> + return treefs_s_file_exec (cred, task, flags, argv, argv_len, envp,
> envp_len,
> + fds, fds_len, ports, ports_len, ints, ints_len,
> + dealloc, dealloc_len, destroy, destroy_len);
> +
> +}
> +
> +error_t
> treefs_S_file_get_translator (struct treefs_protid *cred,
> char **trans, unsigned *trans_len)
> {
Don't touch libtreefs.
> diff --git a/libtrivfs/file-exec.c b/libtrivfs/file-exec.c
> index a3ab048..a321f05 100644
> --- a/libtrivfs/file-exec.c
> +++ b/libtrivfs/file-exec.c
> @@ -40,3 +40,29 @@ trivfs_S_file_exec (trivfs_protid_t exec_file,
> {
> return EOPNOTSUPP;
> }
> +
> +kern_return_t
> +trivfs_S_file_exec_file_name (trivfs_protid_t exec_file,
> + mach_port_t reply,
> + mach_msg_type_name_t replyPoly,
> + mach_port_t exec_task,
> + int flags,
> + data_t filename,
> + mach_msg_type_number_t filenameCnt,
> + data_t argv,
> + mach_msg_type_number_t argvCnt,
> + data_t envp,
> + mach_msg_type_number_t envpCnt,
> + portarray_t fdarray,
> + mach_msg_type_number_t fdarrayCnt,
> + portarray_t portarray,
> + mach_msg_type_number_t portarrayCnt,
> + intarray_t intarray,
> + mach_msg_type_number_t intarrayCnt,
> + mach_port_array_t deallocnames,
> + mach_msg_type_number_t deallocnamesCnt,
> + mach_port_array_t destroynames,
> + mach_msg_type_number_t destroynamesCnt)
> +{
> + return EOPNOTSUPP;
> +}
No filenamelen.
> diff --git a/trans/fakeroot.c b/trans/fakeroot.c
> index c110234..acb980b 100644
> --- a/trans/fakeroot.c
> +++ b/trans/fakeroot.c
> @@ -705,6 +705,40 @@ netfs_S_file_exec (struct protid *user,
> mach_port_t *destroynames,
> size_t destroynameslen)
> {
> + return netfs_S_file_exec_file_name (user,
> + task,
> + flags,
> + NULL, 0,
> + argv, argvlen,
> + envp, envplen,
> + fds, fdslen,
> + portarray, portarraylen,
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> +}
> +
> +kern_return_t
> +netfs_S_file_exec_file_name (struct protid *user,
> + task_t task,
> + int flags,
> + char *filename,
> + size_t filenamelen,
> + char *argv,
> + size_t argvlen,
> + char *envp,
> + size_t envplen,
> + mach_port_t *fds,
> + size_t fdslen,
> + mach_port_t *portarray,
> + size_t portarraylen,
> + int *intarray,
> + size_t intarraylen,
> + mach_port_t *deallocnames,
> + size_t deallocnameslen,
> + mach_port_t *destroynames,
> + size_t destroynameslen)
> +{
> error_t err;
> file_t file;
No filenamelen and "" instead of NULL.
> @@ -723,11 +757,15 @@ netfs_S_file_exec (struct protid *user,
> {
> /* We cannot use MACH_MSG_TYPE_MOVE_SEND because we might need to
> retry an interrupted call that would have consumed the rights. */
> - err = file_exec (user->po->np->nn->file, task, flags, argv, argvlen,
> - envp, envplen, fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> - portarray, MACH_MSG_TYPE_COPY_SEND, portarraylen,
> - intarray, intarraylen, deallocnames, deallocnameslen,
> - destroynames, destroynameslen);
> + err = file_exec_file_name (user->po->np->nn->file, task, flags,
> + filename, filenamelen,
> + argv, argvlen,
> + envp, envplen,
> + fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> + portarray, MACH_MSG_TYPE_COPY_SEND,
> portarraylen,
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> mach_port_deallocate (mach_task_self (), file);
> }
>
No filenamelen.
> >From 85033bc73229c2a17ff85d9003e5717e9c340aa4 Mon Sep 17 00:00:00 2001
> From: Emilio Pozuelo Monfort <pochu27@gmail.com>
> Date: Sat, 22 May 2010 18:26:29 +0200
> Subject: [PATCH] Use the new __hurd_exec_file_name RPC
>
> This fixes problems when an script could env with /dev/fd/N
> in argv[0] because the exec server didn't know the file name.
> ---
> hurd/Versions | 1 +
> hurd/hurd.h | 10 +++++++++-
> hurd/hurdexec.c | 29 +++++++++++++++++++++--------
> sysdeps/mach/hurd/execve.c | 3 ++-
> 4 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/hurd/Versions b/hurd/Versions
> index 83c8ab1..81cd904 100644
> --- a/hurd/Versions
> +++ b/hurd/Versions
> @@ -73,6 +73,7 @@ libc {
> _hurd_critical_section_unlock;
> _hurd_exception2signal;
> _hurd_exec;
> + _hurd_exec_file_name;
> _hurd_fd_get;
> _hurd_init;
> _hurd_intern_fd;
Oh, I haven't noticed this file before. :-)
I don't really know how this file works but this seems to be a section
devoted to compatibility to an older version of glibc so I doubt this is
the correct place to put a new function. You might want to ask Samuel
or Thomas about it, or if that fails, libc-alpha and Roland.
> diff --git a/hurd/hurd.h b/hurd/hurd.h
> index 642ea43..361fb0f 100644
> --- a/hurd/hurd.h
> +++ b/hurd/hurd.h
> @@ -243,13 +243,21 @@ extern FILE *fopenport (io_t port, const char *mode);
> 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. */
I guess that's one step closer to a colon. ;-)
>
> extern error_t _hurd_exec (task_t task,
> file_t file,
> char *const argv[],
> char *const envp[]);
>
> +/* 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
> task thoroughly. This function never returns, no matter what. */
Good.
> diff --git a/hurd/hurdexec.c b/hurd/hurdexec.c
> index beae869..beb41de 100644
> --- a/hurd/hurdexec.c
> +++ b/hurd/hurdexec.c
> @@ -32,11 +32,22 @@
>
> /* 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. */
Colon.
> 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);
> +}
> +
> +/* 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. */
> +error_t
> +_hurd_exec_file_name (task_t task, file_t file, const char *filename,
> + char *const argv[], char *const envp[])
> +{
> error_t err;
> char *args, *env;
> size_t argslen, envlen;
Using NULL is actually reasonable at this level.
Mention filename and what happens if it's NULL in comment.
> @@ -362,13 +373,15 @@ _hurd_exec (task_t task, file_t file,
> if (__sigismember (&_hurdsig_traced, SIGKILL))
> flags |= EXEC_SIGTRAP;
> #endif
> - err = __file_exec (file, task, flags,
> - args, argslen, env, envlen,
> - dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
> - ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,
> - ints, INIT_INT_MAX,
> - please_dealloc, pdp - please_dealloc,
> - &_hurd_msgport, task == __mach_task_self () ? 1 : 0);
> + err = __file_exec_file_name (file, task, flags,
> + filename, filename ? strlen (filename) + 1 :
> 0,
> + args, argslen, env, envlen,
> + dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
> + ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,
> + ints, INIT_INT_MAX,
> + please_dealloc, pdp - please_dealloc,
> + &_hurd_msgport,
> + task == __mach_task_self () ? 1 : 0);
> }
Pass "" if filename is NULL.
Drop filenamelen.
> /* Release references to the standard ports. */
> diff --git a/sysdeps/mach/hurd/execve.c b/sysdeps/mach/hurd/execve.c
> index 8af8b33..f21440f 100644
> --- a/sysdeps/mach/hurd/execve.c
> +++ b/sysdeps/mach/hurd/execve.c
> @@ -35,7 +35,8 @@ __execve (file_name, argv, envp)
> return -1;
>
> /* Hopefully this will not return. */
> - err = _hurd_exec (__mach_task_self (), file, argv, envp);
> + err = _hurd_exec_file_name (__mach_task_self (), file,
> + file_name, argv, envp);
>
> /* Oh well. Might as well be tidy. */
> __mach_port_deallocate (__mach_task_self (), file);
Good.
There, that should be all of it. :-)
Regards,
Fredrik