[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] Add a file_exec_file_name RPC
|
From: |
Carl Fredrik Hammar |
|
Subject: |
Re: [PATCH 2/3] Add a file_exec_file_name RPC |
|
Date: |
Mon, 31 May 2010 18:11:22 +0200 |
|
User-agent: |
Mutt/1.5.20 (2009-06-14) |
Hi,
On Wed, May 26, 2010 at 01:27:40AM +0200, Emilio Pozuelo Monfort wrote:
> diff --git a/exec/hashexec.c b/exec/hashexec.c
> index 6be8dfe..c2d0f7d 100644
> --- a/exec/hashexec.c
> +++ b/exec/hashexec.c
> @@ -420,15 +420,28 @@ check_hashbang (struct execdata *e,
> return;
>
> /* Execute the interpreter program. */
> - e->error = file_exec (interp_file,
> - oldtask, flags,
> - new_argv, new_argvlen, envp, envplen,
> - new_dtable ?: dtable, MACH_MSG_TYPE_COPY_SEND,
> - new_dtable ? new_dtablesize : dtablesize,
> - portarray, MACH_MSG_TYPE_COPY_SEND, nports,
> - intarray, nints,
> - deallocnames, ndeallocnames,
> - destroynames, ndestroynames);
> + e->error = file_exec_file_name (interp_file,
> + oldtask, flags, interp,
> + new_argv, new_argvlen, envp, envplen,
> + new_dtable ?: dtable, MACH_MSG_TYPE_COPY_SEND,
Break this line.
> + new_dtable ? new_dtablesize : dtablesize,
> + portarray, MACH_MSG_TYPE_COPY_SEND, nports,
> + intarray, nints,
> + deallocnames, ndeallocnames,
> + destroynames, ndestroynames);
> + /* For backwards compatibility. Just drop it when we kill file_exec. */
> + if (e->error == MIG_BAD_ID)
> + e->error = file_exec (interp_file,
> + oldtask, flags,
> + new_argv, new_argvlen, envp, envplen,
> + new_dtable ?: dtable, MACH_MSG_TYPE_COPY_SEND,
> + new_dtable ? new_dtablesize : dtablesize,
> + portarray, MACH_MSG_TYPE_COPY_SEND, nports,
> + intarray, nints,
> + deallocnames, ndeallocnames,
> + destroynames, ndestroynames);
> +
> +
> mach_port_deallocate (mach_task_self (), interp_file);
>
> if (! e->error)
Good, not we can handle the case where the interpreter is interpreted
and relies on a correct file argument. :-)
> diff --git a/hurd/fs.defs b/hurd/fs.defs
> index 52d83bd..ac36490 100644
> --- a/hurd/fs.defs
> +++ b/hurd/fs.defs
> @@ -1,5 +1,5 @@
> /* Definitions for the filesystem interface.
> - Copyright (C) 1994,95,96,97,98,99,2002 Free Software Foundation, Inc.
> + Copyright (C) 1994,95,96,97,98,99,2002,10 Free Software Foundation, Inc.
>
> This file is part of the GNU Hurd.
>
Reformat.
> diff --git a/hurd/hurd_types.h b/hurd/hurd_types.h
> index 86b9bcb..999af6f 100644
> --- a/hurd/hurd_types.h
> +++ b/hurd/hurd_types.h
> @@ -1,5 +1,5 @@
> /* C declarations for Hurd server interfaces
> - Copyright (C) 1993,94,95,96,98,99,2001,02 Free Software Foundation, Inc.
> + Copyright (C) 1993,94,95,96,98,99,2001,02,10 Free Software Foundation,
> Inc.
>
> This file is part of the GNU Hurd.
Reformat.
> diff --git a/init/init.c b/init/init.c
> index 4645729..8694f16 100644
> --- a/init/init.c
> +++ b/init/init.c
> @@ -375,13 +375,22 @@ run (const char *server, mach_port_t *ports, task_t
> *task)
> printf ("Pausing for %s\n", prog);
> getchar ();
> }
> - err = file_exec (file, *task, 0,
> - (char *)prog, strlen (prog) + 1, /* Args. */
> - startup_envz, startup_envz_len,
> - default_dtable, MACH_MSG_TYPE_COPY_SEND, 3,
> - ports, MACH_MSG_TYPE_COPY_SEND, INIT_PORT_MAX,
> - default_ints, INIT_INT_MAX,
> - NULL, 0, NULL, 0);
> + err = file_exec_file_name (file, *task, 0, (char *)prog,
> + (char *)prog, strlen (prog) + 1, /* Args.
> */
Break this line.
> + startup_envz, startup_envz_len,
> + default_dtable, MACH_MSG_TYPE_COPY_SEND, 3,
Break this line.
> + ports, MACH_MSG_TYPE_COPY_SEND,
> INIT_PORT_MAX,
Break this line.
> + default_ints, INIT_INT_MAX,
> + NULL, 0, NULL, 0);
> + /* Fallback in case the file server hasn't been restarted yet. */
> + if (err == MIG_BAD_ID)
> + err = file_exec (file, *task, 0,
> + (char *)prog, strlen (prog) + 1, /* Args. */
> + startup_envz, startup_envz_len,
> + default_dtable, MACH_MSG_TYPE_COPY_SEND, 3,
> + ports, MACH_MSG_TYPE_COPY_SEND, INIT_PORT_MAX,
> + default_ints, INIT_INT_MAX,
> + NULL, 0, NULL, 0);
> if (!err)
> break;
>
The comment will likely confuse people. The variant you used above was
much better:
/* For backwards compatibility. Just drop it when we kill file_exec. */
> @@ -468,14 +477,25 @@ run_for_real (char *filename, char *args, int arglen,
> mach_port_t ctty,
> ++progname;
> else
> progname = filename;
> - err = file_exec (file, task, 0,
> - args, arglen,
> - startup_envz, startup_envz_len,
> - default_dtable, MACH_MSG_TYPE_COPY_SEND, 3,
> - default_ports, MACH_MSG_TYPE_COPY_SEND,
> - INIT_PORT_MAX,
> - default_ints, INIT_INT_MAX,
> - NULL, 0, NULL, 0);
> + err = file_exec_file_name (file, task, 0, filename,
> + args, arglen,
> + startup_envz, startup_envz_len,
> + default_dtable, MACH_MSG_TYPE_COPY_SEND, 3,
> + default_ports, MACH_MSG_TYPE_COPY_SEND,
> + INIT_PORT_MAX,
> + default_ints, INIT_INT_MAX,
> + NULL, 0, NULL, 0);
> + /* Fallback in case the file server hasn't been restarted yet. */
> + if (err == MIG_BAD_ID)
> + err = file_exec (file, task, 0,
> + args, arglen,
> + startup_envz, startup_envz_len,
> + default_dtable, MACH_MSG_TYPE_COPY_SEND, 3,
> + default_ports, MACH_MSG_TYPE_COPY_SEND,
> + INIT_PORT_MAX,
> + default_ints, INIT_INT_MAX,
> + NULL, 0, NULL, 0);
> +
Use other comment variant.
> @@ -1059,13 +1079,23 @@ start_child (const char *prog, char **progargs)
> getchar ();
> }
>
> - err = file_exec (file, child_task, 0,
> - args, arglen,
> - startup_envz, startup_envz_len,
> - NULL, MACH_MSG_TYPE_COPY_SEND, 0, /* No fds. */
> - default_ports, MACH_MSG_TYPE_COPY_SEND, INIT_PORT_MAX,
> - default_ints, INIT_INT_MAX,
> - NULL, 0, NULL, 0);
> + err = file_exec_file_name (file, child_task, 0, args,
> + args, arglen,
> + startup_envz, startup_envz_len,
> + NULL, MACH_MSG_TYPE_COPY_SEND, 0, /* No fds. */
> + default_ports, MACH_MSG_TYPE_COPY_SEND,
> INIT_PORT_MAX,
Break this line.
> + default_ints, INIT_INT_MAX,
> + NULL, 0, NULL, 0);
> + /* Fallback in case the file server hasn't been restarted yet. */
> + if (err == MIG_BAD_ID)
> + err = file_exec (file, child_task, 0,
> + args, arglen,
> + startup_envz, startup_envz_len,
> + NULL, MACH_MSG_TYPE_COPY_SEND, 0, /* No fds. */
> + default_ports, MACH_MSG_TYPE_COPY_SEND, INIT_PORT_MAX,
> + default_ints, INIT_INT_MAX,
> + NULL, 0, NULL, 0);
> +
> mach_port_deallocate (mach_task_self (), default_ports[INIT_PORT_PROC]);
> mach_port_deallocate (mach_task_self (), file);
> free (args);
Use other comment variant.
> diff --git a/libdiskfs/boot-start.c b/libdiskfs/boot-start.c
> index ad3cf1a..ba391a2 100644
> --- a/libdiskfs/boot-start.c
> +++ b/libdiskfs/boot-start.c
> @@ -277,7 +277,7 @@ diskfs_start_bootstrap ()
> mach_port_deallocate (mach_task_self (), bootpt);
> assert_perror (err);
>
> - /* Cache the exec server port for file_exec to use. */
> + /* Cache the exec server port for file_exec_file_name to use. */
> _hurd_port_set (&_diskfs_exec_portcell, diskfs_exec);
> }
No copyright update but I guess your relying on the 10 lines rule. ;-)
> diff --git a/libdiskfs/file-exec.c b/libdiskfs/file-exec.c
> index 452240c..fb36b14 100644
> --- a/libdiskfs/file-exec.c
> +++ b/libdiskfs/file-exec.c
> @@ -1,5 +1,5 @@
> -/* File execution (file_exec RPC) for diskfs servers, using exec server.
> - Copyright (C) 1993,94,95,96,97,98,2000,02 Free Software Foundation, Inc.
> +/* File execution (file_exec_file_name RPC) for diskfs servers, using exec
> server.
> + Copyright (C) 1993,94,95,96,97,98,2000,02,10 Free Software Foundation,
> Inc.
>
> This file is part of the GNU Hurd.
>
Reformat.
> @@ -137,8 +170,8 @@ diskfs_S_file_exec (struct protid *cred,
> if (! err)
> /* Make a new peropen for the exec server to access the file, since any
> seeking the exec server might want to do should not affect the
> - original peropen on which file_exec was called. (The new protid for
> - this peropen clones the caller's iouser to preserve the caller's
> + original peropen on which file_exec_file_name was called. (The new
> protid
> + for this peropen clones the caller's iouser to preserve the caller's
> authentication credentials.) The new peropen's openmodes must have
> O_READ even if the caller had only O_EXEC privilege, so the exec
> server can read the executable file. We also include O_EXEC so that
Hmm, I dislike that one line is longer than 80 characters but reformatting
the entire paragraph seems excessive. How about this:
- seeking the exec server might want to do should not affect the
- original peropen on which file_exec was called. (The new protid for
- this peropen clones the caller's iouser to preserve the caller's
+ seeking the exec server might want to do should not affect the original
+ peropen on which file_exec_file_name was called. (The new protid
+ for this peropen clones the caller's iouser to preserve the caller's
> @@ -159,14 +192,27 @@ 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,
> + argv, argvlen, envp, envplen,
> + fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> + portarray, MACH_MSG_TYPE_COPY_SEND,
> portarraylen,
Break this line.
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> + /* Fallback in case the exec server hasn't been restarted. */
> + if (err == MIG_BAD_ID)
> + 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);
> +
> +
> mach_port_deallocate (mach_task_self (), right);
> if (err == MACH_SEND_INVALID_DEST)
> {
Otherwise good.
> diff --git a/libfshelp/start-translator-long.c
> b/libfshelp/start-translator-long.c
> index 5bf1454..368ad54 100644
> --- a/libfshelp/start-translator-long.c
> +++ b/libfshelp/start-translator-long.c
> @@ -1,5 +1,5 @@
> /*
> - Copyright (C) 1995,96,99,2000,02, 04 Free Software Foundation, Inc.
> + Copyright (C) 1995,96,99,2000,02,04,10 Free Software Foundation, Inc.
> Written by Miles Bader and Michael I. Bushnell.
>
> This file is part of the GNU Hurd.
Reformat.
> @@ -205,8 +205,7 @@ fshelp_start_translator_long (fshelp_open_fn_t
> underlying_open_fn,
> mach_port_t prev_notify, proc, saveport, childproc;
> int ports_moved = 0;
>
> - /* Find the translator itself. Since argz has zero-separated elements, we
> - can use it as a normal string representing the first element. */
> + /* Find the translator itself. */
> executable = file_name_lookup(name, O_EXEC, 0);
> if (executable == MACH_PORT_NULL)
> return errno;
This is an unrelated change and should go into its own patch, and be
committed immediately. Good catch though! It has actually been like this
since the first revision of the file.
> @@ -265,11 +264,19 @@ fshelp_start_translator_long (fshelp_open_fn_t
> underlying_open_fn,
> ports[INIT_PORT_BOOTSTRAP] = bootstrap;
>
> /* Try and exec the translator in TASK... */
> - err = file_exec (executable, task, EXEC_DEFAULTS,
> - argz, argz_len, 0, 0,
> - fds, fds_type, fds_len,
> - ports, ports_type, ports_len,
> - ints, ints_len, 0, 0, 0, 0);
> + err = file_exec_file_name (executable, task, EXEC_DEFAULTS, name,
> + argz, argz_len, 0, 0,
> + fds, fds_type, fds_len,
> + ports, ports_type, ports_len,
> + ints, ints_len, 0, 0, 0, 0);
> + /* Fallback in case the file server hasn't been restarted. */
> + if (err == MIG_BAD_ID)
> + err = file_exec (executable, task, EXEC_DEFAULTS,
> + argz, argz_len, 0, 0,
> + fds, fds_type, fds_len,
> + ports, ports_type, ports_len,
> + ints, ints_len, 0, 0, 0, 0);
> +
> ports_moved = 1;
>
> if (ports_type == MACH_MSG_TYPE_COPY_SEND)
Use other comment variant.
> diff --git a/libnetfs/file-exec.c b/libnetfs/file-exec.c
> index 73c125b..ff57934 100644
> --- a/libnetfs/file-exec.c
> +++ b/libnetfs/file-exec.c
> @@ -1,5 +1,5 @@
> /*
> - Copyright (C) 1996,97,2000,01,02 Free Software Foundation, Inc.
> + Copyright (C) 1996,97,2000,01,02,10 Free Software Foundation, Inc.
> Written by Michael I. Bushnell, p/BSG.
>
> This file is part of the GNU Hurd.
Reformat.
> @@ -133,14 +166,26 @@ 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,
> + argv, argvlen, envp, envplen,
> + fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> + portarray, MACH_MSG_TYPE_COPY_SEND,
> portarraylen,
Break this line.
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> + /* Fallback in case the exec server hasn't been restarted. */
Use other comment variant.
> + if (err == MIG_BAD_ID)
> + 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);
> +
> mach_port_deallocate (mach_task_self (), right);
> ports_port_deref (newpi);
> }
Otherwise good.
> diff --git a/libtrivfs/file-exec.c b/libtrivfs/file-exec.c
> index a3ab048..5f40987 100644
> --- a/libtrivfs/file-exec.c
> +++ b/libtrivfs/file-exec.c
> @@ -1,5 +1,5 @@
> /*
> - Copyright (C) 1994,2002 Free Software Foundation, Inc.
> + Copyright (C) 1994,2002,10 Free Software Foundation, Inc.
>
> This program is free software; you can redistribute it and/or
> modify it under the terms of the GNU General Public License as
Reformat.
> diff --git a/trans/fakeroot.c b/trans/fakeroot.c
> index c110234..8ef2626 100644
> --- a/trans/fakeroot.c
> +++ b/trans/fakeroot.c
> @@ -723,11 +756,23 @@ 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,
> + argv, argvlen,
> + envp, envplen,
> + fds, MACH_MSG_TYPE_COPY_SEND, fdslen,
> + portarray, MACH_MSG_TYPE_COPY_SEND,
> portarraylen,
Break this line.
> + intarray, intarraylen,
> + deallocnames, deallocnameslen,
> + destroynames, destroynameslen);
> + /* Fallback in case the file server hasn't been restarted. */
Use other comment variant.
> + if (err == MIG_BAD_ID)
> + 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);
> +
> mach_port_deallocate (mach_task_self (), file);
> }
Otherwise good.
> diff --git a/utils/login.c b/utils/login.c
> index 58c8214..f2f91be 100644
> --- a/utils/login.c
> +++ b/utils/login.c
> @@ -1,6 +1,6 @@
> /* Hurdish login
>
> - Copyright (C) 1995,96,97,98,99,2002 Free Software Foundation, Inc.
> + Copyright (C) 1995,96,97,98,99,2002,2010 Free Software Foundation, Inc.
>
> Written by Miles Bader <miles@gnu.org>
>
Reformat.
Regards,
Fredrik
- Re: exec server and /dev/fd/N, (continued)
- [PATCH] Use the new __hurd_exec_file_name RPC, Emilio Pozuelo Monfort, 2010/05/27
- Re: [PATCH] Use the new __hurd_exec_file_name RPC, Carl Fredrik Hammar, 2010/05/31
- Re: [PATCH] Use the new __hurd_exec_file_name RPC, olafBuddenhagen, 2010/05/31
- [PATCH 3/3] Use the new _hurd_exec_file_name function, Emilio Pozuelo Monfort, 2010/05/27
- Re: [PATCH 3/3] Use the new _hurd_exec_file_name function, Carl Fredrik Hammar, 2010/05/31
- [PATCH 2/3] Add a file_exec_file_name RPC, Emilio Pozuelo Monfort, 2010/05/27
- Re: [PATCH 2/3] Add a file_exec_file_name RPC,
Carl Fredrik Hammar <=
- Re: exec server and /dev/fd/N, Carl Fredrik Hammar, 2010/05/31
- Re: exec server and /dev/fd/N, Carl Fredrik Hammar, 2010/05/31
Re: exec server and /dev/fd/N, olafBuddenhagen, 2010/05/26