[patch]: PR 577 & "#pragma code_page" support for windres

[Dave Korn]

On 06 June 2007 11:47, Kai Tietz wrote:

> Hello,
> 
> I added the support of the --codepage (-c) option to windres tool.
> Additionally windres supports now the "code_page" pragma. It has the
> following syntax:
> #pragma code_page( DEFAULT | <number>)
> 
> May somebody could add a good testcase for the codepage feature.

  I wonder if anyone else can answer, because I don't know: are we allowed to use 'od' in testcases?

> ChangeLog:
> 
> 2007-06-06  Kai Tietz  <Kai.Tietz@onevision.com>
>         * binutils/rclex.c: (cpp_line): Add code_page pragma
>         support.

  There are some lines indented with 8 spaces that should replace them by a single tab.  Also, ...

   char *send, *fn;
+  size_t len, mlen;
 
   ++s;   while (ISSPACE (*s))
     ++s;
+  /* Check for #pragma code_page ( DEFAULT | <nr>).  */

... your patch got slightly mangled there (it also had a stray '=' at the end).

+	    if (ncp == CP_UTF16 || ! unicode_is_valid_codepage (ncp))
+	      fatal (_("invalid value by pragma code_page specified.\n"));

  Should read "invalid value specified for pragma code_page".

>         * binutils/windres.c: (usage, long_options, main):
>         Add new option --codepage & -c.
>         * binutils/winduni.c: (wind_default_codepage,
>         wind_current_codepage): New.
>         (unicode_from_ascii, ascii_from_unicode): Use
>         wind_current_codepage as codepage parameter.
>         (unicode_print): Print 4 characters for hexadecimal
>         values in unicode strings.
>         * binutils/winduni.h:  (wind_default_codepage,
>         wind_current_codepage): New.
>         * binutils/doc/binutils.texi: Add new option.

  BTW, ChangeLog entries should begin each line with a TAB, not spaces.

  I only tested the patch to the extent of verifying that it correctly parsed a bunch of random code_page pragmas without crashing and that it complained about non-existent code pages and accepted valid ones.  Oh, and there were no testsuite regressions.

  The patch looks good to me, bearing in mind the above comments.  I recommend it for approval.

    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....