[prev in list] [next in list] [prev in thread] [next in thread] 

List:       hurd-bug
Subject:    Re: [PATCH] Use the new __hurd_exec_file_name RPC
From:       Carl Fredrik Hammar <hammy.lite () gmail ! com>
Date:       2010-05-31 16:16:06
Message-ID: 20100531161606.GC16581 () mother
[Download RAW message or body]

Hi,

On Sat, May 22, 2010 at 06:26:29PM +0200, Emilio Pozuelo Monfort wrote:
> 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;

Perhaps you missed it in the confusion but this should go into a new section.
Like this:

  GLIBC_2.12.1 {
    # "quasi-internal" functions
    _hurd_exec_file_name;
  }

Or whatever version this gets into.  You should probably note this in
the commit message so it isn't forgotten.

> diff --git a/hurd/hurdexec.c b/hurd/hurdexec.c
> index beae869..fea2618 100644
> --- a/hurd/hurdexec.c
> +++ b/hurd/hurdexec.c
> @@ -362,13 +377,26 @@ _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 : "",
> +				   args, argslen, env, envlen,
> +				   dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
> +				   ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,

Break this line.

> +				   ints, INIT_INT_MAX,
> +				   please_dealloc, pdp - please_dealloc,
> +				   &_hurd_msgport,
> +				   task == __mach_task_self () ? 1 : 0);
> +      /* Fall back for backwards compatibility.  This can just be removed
> +         when __file_exec goes away.  */
> +      if (err)
> +	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);
>      }
>  
>    /* Release references to the standard ports.  */

Only fall back on MIG_BAD_ID.

> diff --git a/sysdeps/mach/hurd/execve.c b/sysdeps/mach/hurd/execve.c
> index 8af8b33..7875c3c 100644
> --- a/sysdeps/mach/hurd/execve.c
> +++ b/sysdeps/mach/hurd/execve.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1991, 92, 93, 94, 95, 97 Free Software Foundation, Inc.
> +/* Copyright (C) 1991, 92, 93, 94, 95, 97, 2010 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or

Break the line.

>     The GNU C Library is free software; you can redistribute it and/or
> @@ -26,8 +26,8 @@
>  int
>  fexecve (int fd, char *const argv[], char *const envp[])
>  {
> -  error_t err = HURD_DPORT_USE (fd, _hurd_exec (__mach_task_self (), port,
> -						argv, envp));
> +  error_t err = HURD_DPORT_USE (fd, _hurd_exec_file_name (__mach_task_self (), port,
> +							  NULL, argv, envp));

Break these lines even more. :-)

> diff --git a/sysdeps/mach/hurd/spawni.c b/sysdeps/mach/hurd/spawni.c
> index 244ca2d..06728a1 100644
> --- a/sysdeps/mach/hurd/spawni.c
> +++ b/sysdeps/mach/hurd/spawni.c
> @@ -60,12 +60,12 @@ __spawni (pid_t *pid, const char *file,
>       that remains visible after an exec is registration with the proc
>       server, and the inheritance of various values and ports.  All those
>       inherited values and ports are what get collected up and passed in the
> -     file_exec RPC by an exec call.  So we do the proc server registration
> +     file_exec_file_name RPC by an exec call.  So we do the proc server registration
>       here, following the model of fork (see fork.c).  We then collect up
>       the inherited values and ports from this (parent) process following
>       the model of exec (see hurd/hurdexec.c), modify or replace each value
>       that fork would (plus the specific changes demanded by ATTRP and
> -     FILE_ACTIONS), and make the file_exec RPC on the requested executable
> +     FILE_ACTIONS), and make the file_exec_file_name RPC on the requested executable
>       file with the child process's task port rather than our own.  This
>       should be indistinguishable from the fork + exec implementation,
>       except that all errors will be detected here (in the parent process)

These are too long.  You should reformat the paragraph.

> @@ -622,14 +620,28 @@ __spawni (pid_t *pid, const char *file,
>  
>      inline error_t exec (file_t file)
>        {
> -	return __file_exec (file, task,
> -			    (__sigismember (&_hurdsig_traced, SIGKILL)
> -			     ? EXEC_SIGTRAP : 0),
> -			    args, argslen, env, envlen,
> -			    dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
> -			    ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,
> -			    ints, INIT_INT_MAX,
> -			    NULL, 0, NULL, 0);
> +	error_t err = __file_exec_file_name (file, task,
> +					     (__sigismember (&_hurdsig_traced, SIGKILL)
> +					      ? EXEC_SIGTRAP : 0),
> +					     filename,
> +					     args, argslen, env, envlen,
> +					     dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
> +					     ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,
> +					     ints, INIT_INT_MAX,
> +					     NULL, 0, NULL, 0);

A bunch of lines should be broken here, but the first one is particularly
tricky.  Do something like this instead:

	error_t err = __file_exec_file_name
	  (file, task,
	   __sigismember (&_hurdsig_traced, SIGKILL) ? EXEC_SIGTRAP : 0,
	   filename,
	   args, argslen, env, envlen,
	   dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
	   ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,
	   ints, INIT_INT_MAX,
	   NULL, 0, NULL, 0);

(I removed the parenthesis surrounding __sigismember() because they're only
there for formatting.)

> +	/* Fallback for backwards compatibility.  This can just be removed
> +	   when __file_exec goes away.  */
> +	if (err)
> +	  return __file_exec (file, task,
> +			      (__sigismember (&_hurdsig_traced, SIGKILL)
> +			      ? EXEC_SIGTRAP : 0),
> +			      args, argslen, env, envlen,
> +			      dtable, MACH_MSG_TYPE_COPY_SEND, dtablesize,
> +			      ports, MACH_MSG_TYPE_COPY_SEND, _hurd_nports,
> +			      ints, INIT_INT_MAX,
> +			      NULL, 0, NULL, 0);
> +
> +	return err;
>        }
>  

Check for MIG_BAD_ID instead.

Regards,
  Fredrik

[prev in list] [next in list] [prev in thread] [next in thread]