Subject: bug#13589: 24.2.92; wrongly escaped call-process arguments

From: Shigeru Fukaya <[email-masked]>
Date: Thu, 31 Jan 2013 01:36:35 +0900

Hi,

`call-process' wrongly treats those multibyte characters whose
2nd byte is 0x5c.
It is bacause sys_spawnve in w32proc.c does bad quoting process.

Bad example:

(let ((coding-system-for-write 'cp932))
  (call-process "sh" nil t nil "-c" "ls d:/tmp/表*"))
ls: cannot access d:/tmp/表*: No such file or directory
2

Correct example:

(let ((coding-system-for-write 'cp932))
  (call-process "sh" nil t nil "-c" "ls d:/tmp/表*"))
d:/tmp/表
d:/tmp/表1

d:/tmp/表表:
abc
0

I tried fixing it (after w32.c as a reference).
Additionally, I set uninitialized `escape_char' to 0.

Note that coding of command lines is different from coding of process inputs.

Regards,
Shigeru

*** w32proc.c	Thu Jan 31 01:06:10 2013
--- w32proc.old.c	Sun Jan 27 11:32:55 2013
***************
*** 1419,1488 ****
    *nptr = NULL;
  }
  
- 
- /* Code copied from max_filename_mbslen in w32.c */
- 
- /* Current codepage for encoding command lines.  */
- static int command_line_codepage;
- 
- /* Return the maximum length in bytes of a multibyte character
-    sequence encoded in the current ANSI codepage.  This is required to
-    correctly walk the encoded file names one character at a time.  */
- static int
- max_cmdline_mbslen (void)
- {
-   /* A simple cache to avoid calling GetCPInfo every time we need to
-      normalize a file name.  The file-name encoding is not supposed to
-      be changed too frequently, if ever.  */
-   static Lisp_Object last_cmdline_encoding;
-   static int last_max_mbslen;
-   Lisp_Object current_encoding;
- 
-   current_encoding = Vcoding_system_for_write;
- 
-   if (!EQ (last_cmdline_encoding, current_encoding))
-     {
-       CPINFO cp_info;
- 
-       last_cmdline_encoding = current_encoding;
-       /* Default to the current ANSI codepage.  */
-       command_line_codepage = w32_ansi_code_page;
-       if (!NILP (current_encoding))
- 	{
- 	  char *cpname = SDATA (SYMBOL_NAME (current_encoding));
- 	  char *cp = NULL, *end;
- 	  int cpnum;
- 
- 	  if (strncmp (cpname, "cp", 2) == 0)
- 	    cp = cpname + 2;
- 	  else if (strncmp (cpname, "windows-", 8) == 0)
- 	    cp = cpname + 8;
- 
- 	  if (cp)
- 	    {
- 	      end = cp;
- 	      cpnum = strtol (cp, &end, 10);
- 	      if (cpnum && *end == '\0' && end - cp >= 2)
- 		command_line_codepage = cpnum;
- 	    }
- 	}
- 
-       if (!command_line_codepage)
- 	command_line_codepage = CP_ACP; /* CP_ACP = 0, but let's not assume that */
- 
-       if (!GetCPInfo (command_line_codepage, &cp_info))
- 	{
- 	  command_line_codepage = CP_ACP;
- 	  if (!GetCPInfo (command_line_codepage, &cp_info))
- 	    emacs_abort ();
- 	}
-       last_max_mbslen = cp_info.MaxCharSize;
-     }
- 
-   return last_max_mbslen;
- }
- 
- 
  /* When a new child process is created we need to register it in our list,
     so intercept spawn requests.  */
  int
--- 1419,1424 ----
***************
*** 1495,1501 ****
    child_process *cp;
    int is_dos_app, is_cygnus_app, is_gui_app;
    int do_quoting = 0;
!   char escape_char = 0;
    /* We pass our process ID to our children by setting up an environment
       variable in their environment.  */
    char ppid_env_var_buffer[64];
--- 1431,1437 ----
    child_process *cp;
    int is_dos_app, is_cygnus_app, is_gui_app;
    int do_quoting = 0;
!   char escape_char;
    /* We pass our process ID to our children by setting up an environment
       variable in their environment.  */
    char ppid_env_var_buffer[64];
***************
*** 1508,1514 ****
       Some extra whitespace characters need quoting in Cygwin programs,
       so this list is conditionally modified below.  */
    char *sepchars = " \t*?";
-   int dbcs_p = max_cmdline_mbslen () > 1;
  
    /* We don't care about the other modes */
    if (mode != _P_NOWAIT)
--- 1444,1449 ----
***************
*** 1622,1628 ****
  
        if (*p == 0)
  	need_quotes = 1;
!       for ( ; *p; p = !dbcs_p ? p + 1 : CharNextExA (command_line_codepage, p, 0))
  	{
  	  if (escape_char == '"' && *p == '\\')
  	    /* If it's a Cygwin app, \ needs to be escaped.  */
--- 1557,1563 ----
  
        if (*p == 0)
  	need_quotes = 1;
!       for ( ; *p; p++)
  	{
  	  if (escape_char == '"' && *p == '\\')
  	    /* If it's a Cygwin app, \ needs to be escaped.  */
***************
*** 1674,1680 ****
  
        if (do_quoting)
  	{
! 	  for ( ; *p; p = !dbcs_p ? p + 1 : CharNextExA (command_line_codepage, p, 0))
  	    if ((strchr (sepchars, *p) != NULL) || *p == '"')
  	      need_quotes = 1;
  	}
--- 1609,1615 ----
  
        if (do_quoting)
  	{
! 	  for ( ; *p; p++)
  	    if ((strchr (sepchars, *p) != NULL) || *p == '"')
  	      need_quotes = 1;
  	}
***************
*** 1702,1708 ****
  	      *parg++ = *p++;
  	    }
  #else
! 	  while (*p)
  	    {
  	      if (*p == '"')
  		{
--- 1637,1643 ----
  	      *parg++ = *p++;
  	    }
  #else
! 	  for ( ; *p; p++)
  	    {
  	      if (*p == '"')
  		{
***************
*** 1717,1736 ****
  		}
  	      else if (escape_char == '"' && *p == '\\')
  		*parg++ = '\\';
  
  	      if (*p == escape_char && escape_char != '"')
  		escape_char_run++;
  	      else
  		escape_char_run = 0;
- 	      if (!dbcs_p)
- 		*parg++ = *p++;
- 	      else
- 		{
- 		  char *p1 = CharNextExA (command_line_codepage, p, 0);
- 
- 		  while (p < p1)
- 		    *parg++ = *p++;
- 		}
  	    }
  	  /* double escape chars before enclosing quote */
  	  while (escape_char_run > 0)
--- 1652,1663 ----
  		}
  	      else if (escape_char == '"' && *p == '\\')
  		*parg++ = '\\';
+ 	      *parg++ = *p;
  
  	      if (*p == escape_char && escape_char != '"')
  		escape_char_run++;
  	      else
  		escape_char_run = 0;
  	    }
  	  /* double escape chars before enclosing quote */
  	  while (escape_char_run > 0)