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

[Kai Tietz <Kai dot Tietz at onevision dot com>]

Hi,

Thank you Dave for reviewing.

"Dave Korn" <dave.korn@artimi.com> wrote on 17.06.2007 16:29:35:

> 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, ...

Ok, I replaced them by tab.

>    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).

Hmm..., I didn't got this problem on my side.

> +       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".

I used your term as fatal message text.

> >         * 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 think, this is my mailer. Sorry, but I am not allowed to use another one 
I prefer.

>   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,
 i.A. Kai Tietz



|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-StraÃe 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  KomplementÃrin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-StraÃe 9 â 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - GeschÃftsfÃhrer: 
Ulrike DÃhler, Manuela Kluger

Attachment: optc_codepage.txt
Description: Text document