Re: Emacsclient/server filename quoting error
Thanks for the revised patch for emacsclient. It solves the problem I reported. Francis - Original Message - From: Juanma Barranquero [EMAIL PROTECTED] To: Eli Zaretskii [EMAIL PROTECTED] Cc: [EMAIL PROTECTED]; emacs-pretest-bug@gnu.org; emacs-devel@gnu.org Sent: Friday, December 15, 2006 12:07 PM Subject: Re: Emacsclient/server filename quoting error On 12/15/06, Eli Zaretskii [EMAIL PROTECTED] wrote: Actually, a cleaner way of fixing this would be to have a WINDOWSNT-only wrapper for execvp, called, say w32_execvp, that does TRT with quoting the arguments. You like this one better, then? /L/e/k/t/u ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
Actually, a cleaner way of fixing this would be to have a WINDOWSNT-only wrapper for execvp, called, say w32_execvp, that does TRT with quoting the arguments. You like this one better, then? Since those issues are full of subtleties I suggest that you have a look at the gnulib CVS (at savannah.gnu.org), inspecting the files `execute.c' and `pipe.c': http://cvs.savannah.gnu.org/viewcvs/gnulib/lib/execute.c?rev=1.7root=gnulibview=log http://cvs.savannah.gnu.org/viewcvs/gnulib/lib/pipe.c?rev=1.6root=gnulibview=log Similar code is used in groff too. Werner ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
On 12/10/06, Francis Wright [EMAIL PROTECTED] wrote:
I execute all the following commands from a cmd command
prompt (outside of Emacs).
emacsclient -n -a runemacs TO DO.txt
works correctly if Emacs IS already running, but if it is not
already running then Emacs does not see the filename correctly;
the quotes do not appear to be passed on to runemacs.
The following patch addresses the issue by allocating quoted copies of
any argument containing spaces before calling execvp.
Any objections to install this fix? As it stands, it affects also
non-Windows builds. Is requoting args the right behavior in these
environments?
/L/e/k/t/u
Index: lib-src/emacsclient.c
===
RCS file: /cvsroot/emacs/emacs/lib-src/emacsclient.c,v
retrieving revision 1.98
diff -u -2 -r1.98 emacsclient.c
--- lib-src/emacsclient.c 30 Nov 2006 22:49:38 - 1.98
+++ lib-src/emacsclient.c 15 Dec 2006 10:19:44 -
@@ -310,8 +310,20 @@
if (alternate_editor)
{
- int i = optind - 1;
+ int j, i = optind - 1;
+
#ifdef WINDOWSNT
- argv[i] = (char *)alternate_editor;
+ argv[i] = (char *) alternate_editor;
#endif
+
+ /* Arguments with spaces have been dequoted, so we
+have to requote them before calling execvp. */
+ for (j = i; argv[j]; j++)
+ if (strchr (argv[j], ' '))
+ {
+ char *quoted = alloca (strlen (argv[j]) + 3);
+ sprintf (quoted, \%s\, argv[j]);
+ argv[j] = quoted;
+ }
+
execvp (alternate_editor, argv + i);
message (TRUE, %s: error executing alternate editor \%s\\n,
___
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
Date: Fri, 15 Dec 2006 11:31:18 +0100 From: Juanma Barranquero [EMAIL PROTECTED] Cc: emacs-pretest-bug@gnu.org, Emacs Devel emacs-devel@gnu.org I execute all the following commands from a cmd command prompt (outside of Emacs). emacsclient -n -a runemacs TO DO.txt works correctly if Emacs IS already running, but if it is not already running then Emacs does not see the filename correctly; the quotes do not appear to be passed on to runemacs. The following patch addresses the issue by allocating quoted copies of any argument containing spaces before calling execvp. Any objections to install this fix? As it stands, it affects also non-Windows builds. Is requoting args the right behavior in these environments? Quoting arguments passed to execvp is _definitely_ the wrong thing for Posix platforms. The fact that we need to do that for Windows is due to the broken implementation of exec* routines in the Microsoft libraries: they concatenate the arguments together without quoting special characters, and pass the result to CreateProcess, with predictably bad results. By contrast, Posix execvp passes the arguments directly into the argv[] array of the child process. So please make this change conditioned on WINDOWSNT. Actually, a cleaner way of fixing this would be to have a WINDOWSNT-only wrapper for execvp, called, say w32_execvp, that does TRT with quoting the arguments. Then you could say #ifdef WINDOWSNT #define execvp w32_execvp #endif in emacsclient.c, and leave the mainline code intact. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
On 12/15/06, Eli Zaretskii [EMAIL PROTECTED] wrote:
Actually, a cleaner way of fixing this would be to have a
WINDOWSNT-only wrapper for execvp, called, say w32_execvp, that does
TRT with quoting the arguments.
You like this one better, then?
/L/e/k/t/u
Index: lib-src/emacsclient.c
===
RCS file: /cvsroot/emacs/emacs/lib-src/emacsclient.c,v
retrieving revision 1.98
diff -u -2 -r1.98 emacsclient.c
--- lib-src/emacsclient.c 30 Nov 2006 22:49:38 - 1.98
+++ lib-src/emacsclient.c 15 Dec 2006 11:49:57 -
@@ -299,4 +299,35 @@
+#ifdef WINDOWSNT
+
+/*
+ execvp() wrapper for Windows.
+ Quotes arguments with embedded spaces.
+*/
+int
+w32_execvp (path, argv)
+ char *path;
+ char **argv;
+{
+ int i;
+
+ argv[0] = (char *) alternate_editor;
+
+ for (i = 0; argv[i]; i++)
+if (strchr (argv[i], ' '))
+ {
+ char *quoted = alloca (strlen (argv[i]) + 3);
+ sprintf (quoted, \%s\, argv[i]);
+ argv[i] = quoted;
+ }
+
+ return execvp (path, argv);
+}
+
+#undef execvp
+#define execvp w32_execvp
+
+#endif /* WINDOWSNT */
+
/*
Try to run a different command, or --if no alternate editor is
@@ -311,7 +342,5 @@
{
int i = optind - 1;
-#ifdef WINDOWSNT
- argv[i] = (char *)alternate_editor;
-#endif
+
execvp (alternate_editor, argv + i);
message (TRUE, %s: error executing alternate editor \%s\\n,
___
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
Date: Fri, 15 Dec 2006 13:07:01 +0100 From: Juanma Barranquero [EMAIL PROTECTED] Cc: [EMAIL PROTECTED], emacs-pretest-bug@gnu.org, emacs-devel@gnu.org On 12/15/06, Eli Zaretskii [EMAIL PROTECTED] wrote: Actually, a cleaner way of fixing this would be to have a WINDOWSNT-only wrapper for execvp, called, say w32_execvp, that does TRT with quoting the arguments. You like this one better, then? Yes, thanks. However, please mention the Windows bug with execvp in the comment to w32_execvp, otherwise it is not clear to the uninitiated why it is needed, and why we quote the arguments we pass to the real execvp. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
On 12/15/06, Werner LEMBERG [EMAIL PROTECTED] wrote: Since those issues are full of subtleties I suggest that you have a look at the gnulib CVS (at savannah.gnu.org), inspecting the files `execute.c' and `pipe.c': Thanks, but I think I'll wait for people to test the current implementation. I don't think Windows users will have very sophisticate or uncommon alternate editor setups and if they do, they must be prepared for the possibility of it not working ;) /L/e/k/t/u ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
On 12/15/06, Eli Zaretskii [EMAIL PROTECTED] wrote: However, please mention the Windows bug with execvp in the comment to w32_execvp I've shamelessly stolen your comment from a previous message. Thanks, /L/e/k/t/u ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
Date: Fri, 15 Dec 2006 16:42:57 +0100 From: Juanma Barranquero [EMAIL PROTECTED] Cc: [EMAIL PROTECTED], emacs-devel@gnu.org, emacs-pretest-bug@gnu.org I've shamelessly stolen your comment from a previous message. That's okay, since I have a copyright assignment on file with the FSF. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
Date: Fri, 15 Dec 2006 16:14:38 +0100 (CET) Cc: [EMAIL PROTECTED], [EMAIL PROTECTED], emacs-devel@gnu.org, emacs-pretest-bug@gnu.org From: Werner LEMBERG [EMAIL PROTECTED] Since those issues are full of subtleties I suggest that you have a look at the gnulib CVS (at savannah.gnu.org), inspecting the files `execute.c' and `pipe.c': http://cvs.savannah.gnu.org/viewcvs/gnulib/lib/execute.c?rev=1.7root=gnulibview=log http://cvs.savannah.gnu.org/viewcvs/gnulib/lib/pipe.c?rev=1.6root=gnulibview=log Thanks, but I see nothing in these source files that could shed any light on the case in point. Similar code is used in groff too. Groff builds a pipe of several programs and then runs that pipe. By contrast, emacsclient needs to run a single program, and doesn't need to read/write its standard streams. So the complexity of the above URLs is not needed. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Emacsclient/server filename quoting error
On 12/10/06, Francis Wright [EMAIL PROTECTED] wrote: I execute all the following commands from a cmd command prompt (outside of Emacs). emacsclient -n -a runemacs TO DO.txt works correctly if Emacs IS already running, but if it is not already running then Emacs does not see the filename correctly; the quotes do not appear to be passed on to runemacs. Yeah. With an instrumented emacsclient: C:\emacs emacsclient -a c:\emacs\bin\runemacs.exe TO DO.txt 2: [c:\emacs\bin\runemacs.exe] 3: [TO DO.txt] So it would be necessary to re-quote args containing whitespaces before calling execvp(). Question is: is that the right behavior in non-Windows environments? (Quoting filenames with embedded spaces, I mean). /L/e/k/t/u ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
