bug#16599: src/temacs --daemon fails

bug#16599: src/temacs --daemon fails

[Alex Bennée]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex Bennée <kernel-hacker@bennee.com>
>> Date: Thu, 30 Jan 2014 14:10:45 +0000
>> 
>> In an unrelated issue I found that I can't start src/temacs with the
>> --daemon option which works with the dumped version src/emacs.
>
> Please use "M-x report-emacs-bug RET" to report such bugs, then they
> are automatically emailed to the bug tracker address.

I raised bug #16599 and I have tracked it down to being reset by
syms_of_emacs() which is called in temacs after being set up by
--daemon. I assume the dumped src/emacs behaves differently.

The following patch works for me:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix initialistion of daemon_pipes --]
[-- Type: text/x-diff, Size: 1039 bytes --]

From 3dee0d9da394e17b4e6cb97cb22399f027cab440 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex@bennee.com>
Date: Sun, 16 Feb 2014 20:59:06 +0000
Subject: [PATCH] src/emacs.c: ensure daemon_pipe initialised before use

Otherwise this breaks src/temacs --daemon invocations by resetting the
daemon_pipe FDs which are used to determine if Emacs is in daemon mode.
---
 src/emacs.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/emacs.c b/src/emacs.c
index 18f6a08..66f47ef 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -201,7 +201,7 @@ static char *daemon_name;
 
 /* Pipe used to send exit notification to the daemon parent at
    startup.  */
-int daemon_pipe[2];
+int daemon_pipe[2] = {0, 0};
 
 /* Save argv and argc.  */
 char **initial_argv;
@@ -2548,7 +2548,4 @@ libraries; only those already known by Emacs will be loaded.  */);
   Vlibrary_cache = Qnil;
   staticpro (&Vlibrary_cache);
 #endif
-
-  /* Make sure IS_DAEMON starts up as false.  */
-  daemon_pipe[1] = 0;
 }
-- 
1.8.5.3


[-- Attachment #3: Type: text/plain, Size: 33 bytes --]



>
> Thanks.

-- 
Alex Bennée

bug#16599: src/temacs --daemon fails

[Andreas Schwab]

Alex Bennée <kernel-hacker@bennee.com> writes:

> diff --git a/src/emacs.c b/src/emacs.c
> index 18f6a08..66f47ef 100644
> --- a/src/emacs.c
> +++ b/src/emacs.c
> @@ -201,7 +201,7 @@ static char *daemon_name;
>  
>  /* Pipe used to send exit notification to the daemon parent at
>     startup.  */
> -int daemon_pipe[2];
> +int daemon_pipe[2] = {0, 0};

This is redundant.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#16599: src/temacs --daemon fails

[Alex Bennée]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Alex Bennée <kernel-hacker@bennee.com> writes:
>
>> diff --git a/src/emacs.c b/src/emacs.c
>> index 18f6a08..66f47ef 100644
>> --- a/src/emacs.c
>> +++ b/src/emacs.c
>> @@ -201,7 +201,7 @@ static char *daemon_name;
>>  
>>  /* Pipe used to send exit notification to the daemon parent at
>>     startup.  */
>> -int daemon_pipe[2];
>> +int daemon_pipe[2] = {0, 0};
>
> This is redundant.

Indeed. I wasn't sure if the code I replaced relied on some special
difference between .data and .bss sections. I have to admit I'm
unfamiliar with how the Emacs dumping mechanism works.

Why did the syms_of_emacs() function feel the need to initialise daemon_pipe?

>
> Andreas.

-- 
Alex Bennée

bug#16599: src/temacs --daemon fails

[Eli Zaretskii]

> From: Alex Bennée <kernel-hacker@bennee.com>
> Cc: Alex Bennée <kernel-hacker@bennee.com>, Eli Zaretskii
>  <eliz@gnu.org>, 16599@debbugs.gnu.org, emacs-devel@gnu.org
> Date: Mon, 17 Feb 2014 07:40:35 +0000
> 
> Why did the syms_of_emacs() function feel the need to initialise daemon_pipe?

Because in a dumped Emacs, daemon_pipe might hold a stale value from
before the dumping.

Does the patch below solve this problem?

=== modified file 'src/emacs.c'
--- src/emacs.c	2014-01-20 19:44:56 +0000
+++ src/emacs.c	2014-02-17 15:48:44 +0000
@@ -2550,5 +2550,6 @@ libraries; only those already known by E
 #endif
 
   /* Make sure IS_DAEMON starts up as false.  */
-  daemon_pipe[1] = 0;
+  if (initialized)
+    daemon_pipe[1] = 0;
 }

bug#16599: src/temacs --daemon fails

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex Bennée <kernel-hacker@bennee.com>
>> Cc: Alex Bennée <kernel-hacker@bennee.com>, Eli Zaretskii
>>  <eliz@gnu.org>, 16599@debbugs.gnu.org, emacs-devel@gnu.org
>> Date: Mon, 17 Feb 2014 07:40:35 +0000
>> 
>> Why did the syms_of_emacs() function feel the need to initialise daemon_pipe?
>
> Because in a dumped Emacs, daemon_pipe might hold a stale value from
> before the dumping.

But syms_of_emacs is not called in a dumped Emacs.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#16599: src/temacs --daemon fails

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: Alex Bennée <kernel-hacker@bennee.com>,
>   16599@debbugs.gnu.org
> Date: Mon, 17 Feb 2014 17:03:01 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Alex Bennée <kernel-hacker@bennee.com>
> >> Cc: Alex Bennée <kernel-hacker@bennee.com>, Eli Zaretskii
> >>  <eliz@gnu.org>, 16599@debbugs.gnu.org, emacs-devel@gnu.org
> >> Date: Mon, 17 Feb 2014 07:40:35 +0000
> >> 
> >> Why did the syms_of_emacs() function feel the need to initialise daemon_pipe?
> >
> > Because in a dumped Emacs, daemon_pipe might hold a stale value from
> > before the dumping.
> 
> But syms_of_emacs is not called in a dumped Emacs.

So I guess we should move that line somewhere where it will be called.

bug#16599: src/temacs --daemon fails

[Eli Zaretskii]

> Date: Mon, 17 Feb 2014 18:15:58 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 16599@debbugs.gnu.org, kernel-hacker@bennee.com
> 
> > From: Andreas Schwab <schwab@suse.de>
> > Cc: Alex Bennée <kernel-hacker@bennee.com>,
> >   16599@debbugs.gnu.org
> > Date: Mon, 17 Feb 2014 17:03:01 +0100
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > >> From: Alex Bennée <kernel-hacker@bennee.com>
> > >> Cc: Alex Bennée <kernel-hacker@bennee.com>, Eli Zaretskii
> > >>  <eliz@gnu.org>, 16599@debbugs.gnu.org, emacs-devel@gnu.org
> > >> Date: Mon, 17 Feb 2014 07:40:35 +0000
> > >> 
> > >> Why did the syms_of_emacs() function feel the need to initialise daemon_pipe?
> > >
> > > Because in a dumped Emacs, daemon_pipe might hold a stale value from
> > > before the dumping.
> > 
> > But syms_of_emacs is not called in a dumped Emacs.
> 
> So I guess we should move that line somewhere where it will be called.

Like in the patch below.  Alex, please try this one instead of the one
I sent before.

=== modified file 'src/emacs.c'
--- src/emacs.c	2014-01-20 19:44:56 +0000
+++ src/emacs.c	2014-02-17 16:36:32 +0000
@@ -721,6 +721,9 @@ main (int argc, char **argv)
   /* If we use --chdir, this records the original directory.  */
   char *original_pwd = 0;
 
+  /* Make sure IS_DAEMON starts up as false.  */
+  daemon_pipe[1] = 0;
+
 #if GC_MARK_STACK
   stack_base = &dummy;
 #endif
@@ -2548,7 +2551,4 @@ libraries; only those already known by E
   Vlibrary_cache = Qnil;
   staticpro (&Vlibrary_cache);
 #endif
-
-  /* Make sure IS_DAEMON starts up as false.  */
-  daemon_pipe[1] = 0;
 }