[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#19688: [patch] add support for emacs daemon on Windows
From: |
Mark Laws |
Subject: |
bug#19688: [patch] add support for emacs daemon on Windows |
Date: |
Mon, 26 Jan 2015 16:40:10 +0900 |
On Mon, Jan 26, 2015 at 3:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> [Please keep the bug address on the CC list, so that this whole
> discussion gets archived.]
Oops, blindly hit reply last time without noticing where it was going
to--sorry about that.
>> Date: Mon, 26 Jan 2015 08:16:38 +0900
>> From: Mark Laws <mdl@60hz.org>
>>
>> >> +#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"
>> >
>> > Where did this GUID come from?
>> >
>> I generated it myself.
>
> Is that safe? Do we care whether this GUID is globally unique? Why
> exactly do we need it to begin with?
It should be safe. On UNIX, Emacs uses a pipe to tell emacsclient when
it's done initializing. On Windows, since we don't have fork, the
easiest options are either a named event object[1] or specifying that
the child process inherit the event handle in CreateProcess. The
former is simpler, so I went with it. We could call it
"EmacsDaemonEvent" or something instead; it doesn't really matter as
long as it's a name nothing else is likely to use.
[1]
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682396%28v=vs.85%29.aspx
>> > Why do we need anything beyond the event handle? Can't it serve
>> > double duty as a flag as well? We could use INVALID_HANDLE_VALUE
>> > and/or NULL as distinct values with specific meanings.
>>
>> I've reworked it to use only w32_daemon_event. One minor issue: to
>> avoid including any Windows-related headers in lisp.h, the extern
>> declaration of w32_daemon_event there is void* instead of HANDLE. I
>> doubt the type of HANDLE will ever change, so it shouldn't matter
>> much.
>
> That should be fine, but there's no need to have the handle in lisp.h,
> either. We could put it on w32.h, for example, which is already
> included by emacs.c and other files. If w32.h doesn't fit the bill,
> please try to find some other w32*.h which does.
IS_DAEMON is defined in lisp.h, so it seems to make the most sense to
put it there rather than having a different IS_DAEMON for Windows in
another place. We'd have to #include a w32 header just for IS_DAEMON
in a lot more files if we move it to one of them, too.
>> >> +#ifndef WINDOWSNT
>> >> /* Make sure IS_DAEMON starts up as false. */
>> >> daemon_pipe[1] = 0;
>> >> +#endif
>> >
>> > You should do a similar initialization on WINDOWSNT, to avoid using
>> > the value that was initialized when Emacs was dumped.
>>
>> I'm not sure I understand. Do you mean for w32_daemon_event? If so, it
>> should already always be zero when Emacs starts up.
>
> It might not be zero. Recall that Emacs is run during the build
> process, when it executes some code, and then "dumps itself", which
> involves writing portions of its memory to a disk file that thereafter
> becomes the emacs.exe you run normally. If, for some reason,
> w32_daemon_event is initialized in this first instance, it will not be
> zero in the dumped image.
>
> So we always re-initialize such variables explicitly, instead of
> relying on the linker initializations. You can see that in the
> sources, and that is also the reason why the Posix code initializes
> daemon_pipe[1].
OK, fixed.
>> > Also, the call to daemon_check_preconditions should be outside of the
>> > #ifdef, as it is used on all platforms.
>>
>> daemon_check_preconditions already gets called for both WINDOWSNT and
>> !WINDOWSNT (see the #else). If we move it outside the #ifdef to
>> eliminate having to mention it for both cases, then "nfd" and "err"
>> will also have to be outside the ifdef in order to be compatible with
>> strict pre-C99 compilers that only accept variable declarations at the
>> beginning of blocks.
>
> C99 compliant compiler is a prerequisite in the development version of
> Emacs, so this problem doesn't exist.
Fixed.
I'll attach another patch when we figure out what to do about the
remaining issues.
--
|v\ /\ |\ |< |_ /\ \^| //
- bug#19688: [patch] add support for emacs daemon on Windows, Mark Laws, 2015/01/25
- bug#19688: [patch] add support for emacs daemon on Windows, Eli Zaretskii, 2015/01/25
- Message not available
- bug#19688: [patch] add support for emacs daemon on Windows, Eli Zaretskii, 2015/01/26
- bug#19688: [patch] add support for emacs daemon on Windows,
Mark Laws <=
- bug#19688: [patch] add support for emacs daemon on Windows, Daniel Colascione, 2015/01/26
- bug#19688: [patch] add support for emacs daemon on Windows, Mark Laws, 2015/01/27
- bug#19688: [patch] add support for emacs daemon on Windows, Mark Laws, 2015/01/29
- bug#19688: [patch] add support for emacs daemon on Windows, Eli Zaretskii, 2015/01/30