bug#3772: 23.0.95; Segmentation fault: ffap/image/C-x d

[Chong Yidong]

> $ 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  <jasonr@gnu.org>

        * 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");