Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

[Ken Brown]

On 9/1/2016 1:16 PM, Eli Zaretskii wrote:
> +   However, using the prototype with intptr_t causes GCC to emit
> +   warnings.  Fortunately, execve is not used in the MinGW build, but
> +   the code that references it is still compiled.  */

Wouldn't it be easier to prevent that code from being compiled?  Then 
you could just remove the prototype.  It seems confusing to include a 
prototype and several lines of commentary for a function that's not used.

I think the following would suffice:

--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -146,6 +146,7 @@ disable_address_randomization (void)
  }
  #endif

+#ifndef WINDOWSNT
  /* Execute the program in FILE, with argument vector ARGV and environ
     ENVP.  Return an error number if unsuccessful.  This is like execve
     except it reenables ASLR in the executed program if necessary, and
@@ -170,6 +171,7 @@ emacs_exec_file (char const *file, char *const 
*argv, char *const *envp)

    return err;
  }
+#endif /* not WINDOWSNT */

  /* If FD is not already open, arrange for it to be open with FLAGS.  */
  static void

Ken

Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> Cc: Angelo Graziosi <angelo.graziosi@alice.it>
> Date: Thu, 1 Sep 2016 15:57:00 -0400
> 
> On 9/1/2016 1:16 PM, Eli Zaretskii wrote:
> > +   However, using the prototype with intptr_t causes GCC to emit
> > +   warnings.  Fortunately, execve is not used in the MinGW build, but
> > +   the code that references it is still compiled.  */
> 
> Wouldn't it be easier to prevent that code from being compiled?

It would for the Windows build, but it would add one (actually more,
see below) #ifdef WINDOWSNT into the mainline code.  AFAIR, Paul
(CC'ed) wanted to keep those to a minimum, so I preferred not to ifdef
away the code.

> I think the following would suffice:
> 
> --- a/src/sysdep.c
> +++ b/src/sysdep.c
> @@ -146,6 +146,7 @@ disable_address_randomization (void)
>   }
>   #endif
> 
> +#ifndef WINDOWSNT
>   /* Execute the program in FILE, with argument vector ARGV and environ
>      ENVP.  Return an error number if unsuccessful.  This is like execve
>      except it reenables ASLR in the executed program if necessary, and
> @@ -170,6 +171,7 @@ emacs_exec_file (char const *file, char *const 
> *argv, char *const *envp)
> 
>     return err;
>   }
> +#endif /* not WINDOWSNT */

That whole function is not used on Windows, so if we are to do this,
the following fragment of main in emacs.c should also be ifdefed away:

    /* True if address randomization interferes with memory allocation.  */
  # ifdef __PPC64__
    bool disable_aslr = true;
  # else
    bool disable_aslr = dumping;
  # endif

    if (disable_aslr && disable_address_randomization ())
      {
	/* Set this so the personality will be reverted before execs
	   after this one.  */
	xputenv ("EMACS_HEAP_EXEC=true");

	/* Address randomization was enabled, but is now disabled.
	   Re-execute Emacs to get a clean slate.  */
	execvp (argv[0], argv);

	/* If the exec fails, warn and then try anyway.  */
	perror (argv[0]);
      }

If Paul doesn't mind (nor anyone else), we can certainly do that.

Thanks.

Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

[Angelo Graziosi]

Hi Paul,

Il 02/09/2016 08:53, Eli Zaretskii ha scritto:
>> From: Ken Brown <kbrown@cornell.edu>
>> Cc: Angelo Graziosi <angelo.graziosi@alice.it>
>> Date: Thu, 1 Sep 2016 15:57:00 -0400
>>
>> On 9/1/2016 1:16 PM, Eli Zaretskii wrote:
>>> +   However, using the prototype with intptr_t causes GCC to emit
>>> +   warnings.  Fortunately, execve is not used in the MinGW build, but
>>> +   the code that references it is still compiled.  */
>>
>> Wouldn't it be easier to prevent that code from being compiled?
>
> It would for the Windows build, but it would add one (actually more,
> see below) #ifdef WINDOWSNT into the mainline code.  AFAIR, Paul
> (CC'ed) wanted to keep those to a minimum, so I preferred not to ifdef
> away the code.

if I understand (in short) there is code compiled but not used.. Would 
it better to exclude that code as Ken suggests, then?

May you comment?

Thanks,
  Angelo.

>
>> I think the following would suffice:
>>
>> --- a/src/sysdep.c
>> +++ b/src/sysdep.c
>> @@ -146,6 +146,7 @@ disable_address_randomization (void)
>>   }
>>   #endif
>>
>> +#ifndef WINDOWSNT
>>   /* Execute the program in FILE, with argument vector ARGV and environ
>>      ENVP.  Return an error number if unsuccessful.  This is like execve
>>      except it reenables ASLR in the executed program if necessary, and
>> @@ -170,6 +171,7 @@ emacs_exec_file (char const *file, char *const
>> *argv, char *const *envp)
>>
>>     return err;
>>   }
>> +#endif /* not WINDOWSNT */
>
> That whole function is not used on Windows, so if we are to do this,
> the following fragment of main in emacs.c should also be ifdefed away:
>
>     /* True if address randomization interferes with memory allocation.  */
>   # ifdef __PPC64__
>     bool disable_aslr = true;
>   # else
>     bool disable_aslr = dumping;
>   # endif
>
>     if (disable_aslr && disable_address_randomization ())
>       {
> 	/* Set this so the personality will be reverted before execs
> 	   after this one.  */
> 	xputenv ("EMACS_HEAP_EXEC=true");
>
> 	/* Address randomization was enabled, but is now disabled.
> 	   Re-execute Emacs to get a clean slate.  */
> 	execvp (argv[0], argv);
>
> 	/* If the exec fails, warn and then try anyway.  */
> 	perror (argv[0]);
>       }
>
> If Paul doesn't mind (nor anyone else), we can certainly do that.
>
> Thanks.
>

Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

[Paul Eggert]

I'd rather keep the mainline code as free from such ifdefs as possible.

If the problem is that GCC complains about execvp due to some configuration 
problem on MS-Windows, perhaps you could put a wrapper into w32common.h or some 
similar location, to pacify the compiler. emacsclient.c already uses such a 
wrapper, for another reason.

Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 4 Sep 2016 11:47:16 -0700
> Cc: emacs-devel@gnu.org
> 
> If the problem is that GCC complains about execvp due to some configuration 
> problem on MS-Windows, perhaps you could put a wrapper into w32common.h or some 
> similar location, to pacify the compiler. emacsclient.c already uses such a 
> wrapper, for another reason.

The problem is already solved, so I see no need for a wrapper.

Thanks.

Re: [Emacs-diffs] master 0c94b84: * nt/inc/ms-w32.h (execve) [MINGW_W64]: Make commentary more accurate.

[Angelo Graziosi]

Il 04/09/2016 20:47, Paul Eggert ha scritto:
> I'd rather keep the mainline code as free from such ifdefs as possible.
>
> If the problem is that GCC complains about execvp due to some
> configuration problem on MS-Windows, perhaps you could put a wrapper
> into w32common.h or some similar location, to pacify the compiler.
> emacsclient.c already uses such a wrapper, for another reason.

It would be better to continue to have all those warnings!

I still think that Ken's solution is the most elegant. In any case, 
Eli's patch is better than this proposal...


Angelo