Subject: Re: 23.0.95; Segmentation fault: ffap/image/C-x d

From: Chong Yidong <[email-masked]>
Date: Tue, 07 Jul 2009 18:07:32 -0400

> $ cd /tmp && wget \
> http://jidanni.org/geo/taipower/images/19990716ab67wow.jpg
> $ emacs-snapshot -Q /tmp/19990716ab67wow.jpg
> M-x ffap-bindings
> C-x d
> Segfault...

I can reproduce this.  From bisecting prior revisions, I think the
problem first appeared with this change:

2009-03-24  Jason Rumney  <[email-masked]>

	* fileio.c (Fsubstitute_in_file_name): Always work on a copy.
	Calculate total size precisely.  Decode environment variables
	before substituting.  (Bug#38)

If I revert this change---the CVS diff is shown below---then the problem
goes away.  Though, with such memory problems, that's no guarantee that
this change is directly responsibe for the bug.

Jason, could you quickly double-check this checkin?  Thanks.

Index: fileio.c
===================================================================
RCS file: /sources/emacs/emacs/src/fileio.c,v
retrieving revision 1.650
retrieving revision 1.651
diff -c -r1.650 -r1.651
*** fileio.c	19 Mar 2009 06:26:07 -0000	1.650
--- fileio.c	24 Mar 2009 14:14:54 -0000	1.651
***************
*** 1629,1639 ****
--- 1629,1642 ----
    unsigned char *target = NULL;
    int total = 0;
    int substituted = 0;
+   int multibyte;
    unsigned char *xnm;
    Lisp_Object handler;
  
    CHECK_STRING (filename);
  
+   multibyte = STRING_MULTIBYTE (filename);
+ 
    /* If the file name has special constructs in it,
       call the corresponding file handler.  */
    handler = Ffind_file_name_handler (filename, Qsubstitute_in_file_name);
***************
*** 1641,1648 ****
      return call2 (handler, Qsubstitute_in_file_name, filename);
  
    nm = SDATA (filename);
! #ifdef DOS_NT
    nm = strcpy (alloca (strlen (nm) + 1), nm);
    CORRECT_DIR_SEPS (nm);
    substituted = (strcmp (nm, SDATA (filename)) != 0);
  #endif
--- 1644,1654 ----
      return call2 (handler, Qsubstitute_in_file_name, filename);
  
    nm = SDATA (filename);
!   /* Always work on a copy of the string, in case GC happens during
!      decode of environment variables, causing the original Lisp_String
!      data to be relocated.  */
    nm = strcpy (alloca (strlen (nm) + 1), nm);
+ #ifdef DOS_NT
    CORRECT_DIR_SEPS (nm);
    substituted = (strcmp (nm, SDATA (filename)) != 0);
  #endif
***************
*** 1655,1663 ****
         again.  Important with filenames like "/home/foo//:/hello///there"
         which whould substitute to "/:/hello///there" rather than "/there".  */
      return Fsubstitute_in_file_name
!       (make_specified_string (p, -1, endp - p,
! 			      STRING_MULTIBYTE (filename)));
! 
  
    /* See if any variables are substituted into the string
       and find the total length of their values in `total' */
--- 1661,1667 ----
         again.  Important with filenames like "/home/foo//:/hello///there"
         which whould substitute to "/:/hello///there" rather than "/there".  */
      return Fsubstitute_in_file_name
!       (make_specified_string (p, -1, endp - p, multibyte));
  
    /* See if any variables are substituted into the string
       and find the total length of their values in `total' */
***************
*** 1703,1710 ****
  	/* Get variable value */
  	o = (unsigned char *) egetenv (target);
  	if (o)
! 	  { /* Eight-bit chars occupy upto 2 bytes in multibyte.  */
! 	    total += strlen (o) * (STRING_MULTIBYTE (filename) ? 2 : 1);
  	    substituted = 1;
  	  }
  	else if (*p == '}')
--- 1707,1722 ----
  	/* Get variable value */
  	o = (unsigned char *) egetenv (target);
  	if (o)
! 	  {
! 	    /* Don't try to guess a maximum length - UTF8 can use up to
! 	       four bytes per character.  This code is unlikely to run
! 	       in a situation that requires performance, so decoding the
! 	       env variables twice should be acceptable. Note that
! 	       decoding may cause a garbage collect.  */
! 	    Lisp_Object orig, decoded;
! 	    orig = make_unibyte_string (o, strlen (o));
! 	    decoded = DECODE_FILE (orig);
! 	    total += SBYTES (decoded);
  	    substituted = 1;
  	  }
  	else if (*p == '}')
***************
*** 1762,1782 ****
  	    *x++ = '$';
  	    strcpy (x, target); x+= strlen (target);
  	  }
- 	else if (STRING_MULTIBYTE (filename))
- 	  {
- 	    /* If the original string is multibyte,
- 	       convert what we substitute into multibyte.  */
- 	    while (*o)
- 	      {
- 		int c = *o++;
- 		c = unibyte_char_to_multibyte (c);
- 		x += CHAR_STRING (c, x);
- 	      }
- 	  }
  	else
  	  {
! 	    strcpy (x, o);
! 	    x += strlen (o);
  	  }
        }
  
--- 1774,1795 ----
  	    *x++ = '$';
  	    strcpy (x, target); x+= strlen (target);
  	  }
  	else
  	  {
! 	    Lisp_Object orig, decoded;
! 	    int orig_length, decoded_length;
! 	    orig_length = strlen (o);
! 	    orig = make_unibyte_string (o, orig_length);
! 	    decoded = DECODE_FILE (orig);
! 	    decoded_length = SBYTES (decoded);
! 	    strncpy (x, SDATA (decoded), decoded_length);
! 	    x += decoded_length;
! 
! 	    /* If environment variable needed decoding, return value
! 	       needs to be multibyte.  */
! 	    if (decoded_length != orig_length
! 		|| strncmp (SDATA (decoded), o, orig_length))
! 	      multibyte = 1;
  	  }
        }
  
***************
*** 1789,1795 ****
         need to quote some $ to $$ first.  */
      xnm = p;
  
!   return make_specified_string (xnm, -1, x - xnm, STRING_MULTIBYTE (filename));
  
   badsubst:
    error ("Bad format environment-variable substitution");
--- 1802,1808 ----
         need to quote some $ to $$ first.  */
      xnm = p;
  
!   return make_specified_string (xnm, -1, x - xnm, multibyte);
  
   badsubst:
    error ("Bad format environment-variable substitution");