Re: 23.0.60; Assuming errstring unibyte is incorrect and leads to Emacs crashes

[Chong Yidong]

Could someone review the patch submitted with bug#778 for correctness?
I've excerpted part of the bug report below; the full report can be
viewed at

 http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=778


Dmitry Dzhus <address@hidden> wrote:

> Looks like the code of `report_file_error' function at fileio.c assumes
> errstring is unibyte after `code_convert_string_norecord' call.
>
>     str = strerror (errorno);
>     errstring = code_convert_string_norecord (make_unibyte_string (str,
>                                                                strlen (str)),
>                                           Vlocale_coding_system, 0);
>...
>     /* System error messages are capitalized.  Downcase the initial
>          unless it is followed by a slash.  */
>       if (SREF (errstring, 1) != '/')
>           SSET (errstring, 0, DOWNCASE (SREF (errstring, 0)));
>
> That's not always true. 
>
> The result of `code_convert_string_norecord' results in a multibyte
> string object when `CODING_FOR_UNIBYTE (Vlocale_coding_system)' is not
> true, that is indicated by code in `decode_coding_object' function from
> coding.c:
>
>     if (EQ (dst_object, Qt)
>       || (! NILP (CODING_ATTR_POST_READ (attrs))
>         && NILP (dst_object)))
>     {
>       coding->dst_multibyte = !CODING_FOR_UNIBYTE (coding);
>....
>
> Looks like the issue may be resolved replacing a SSET&SREF call at
> `report_file_error' with something multibyte-aware. I've attached a
> one-line patch which does the job. It makes an assertion that a
> downcase and uppercase character both occupy the same amount of bytes.
>
> Using `Fdowncase' function instead also fixes the problem, but at a
> cost of slight overhead (the whole string gets processed, not just the
> first character (this cannot impact performance seriously though, as I
> believe those `errstring' variables are not processed often)) my patch
> does not introduce.
>
> I've tested the patch with several locales (both problematic and
> traditional ones) and couldn't spot any regression introduced with
> this patch.

*** fileio.c.~1.629.~   2008-08-06 01:41:14.000000000 +0400
--- fileio.c    2008-08-26 02:09:11.000000000 +0400
***************
*** 262,268 ****
        /* System error messages are capitalized.  Downcase the initial
           unless it is followed by a slash.  */
        if (SREF (errstring, 1) != '/')
!         SSET (errstring, 0, DOWNCASE (SREF (errstring, 0)));
  
        xsignal (Qfile_error,
                 Fcons (build_string (string), Fcons (errstring, data)));
--- 262,268 ----
        /* System error messages are capitalized.  Downcase the initial
           unless it is followed by a slash.  */
        if (SREF (errstring, 1) != '/')
!         CHAR_STRING (DOWNCASE (STRING_CHAR (SDATA(errstring), 0)), 
SDATA(errstring));
  
        xsignal (Qfile_error,
                 Fcons (build_string (string), Fcons (errstring, data)));