Re: [PATCH] winemac.drv: Support the public UTF-16 type for Unicode text.

[Ken Thomases]

On Aug 21, 2013, at 9:42 PM, Charles Davis wrote:
> @@ -90,6 +93,9 @@ static CFDataRef export_hdrop_to_filenames(HANDLE data);
> static CFDataRef export_oemtext_to_utf8(HANDLE data);
> static CFDataRef export_text_to_utf8(HANDLE data);
> static CFDataRef export_unicodetext_to_utf8(HANDLE data);
> +static CFDataRef export_oemtext_to_utf16(HANDLE data);
> +static CFDataRef export_text_to_utf16(HANDLE data);
> +static CFDataRef export_unicodetext_to_utf16(HANDLE data);

Please keep the export_* functions sorted (although I'm fine with utf8 coming before utf16).  This applies to the function definitions, too.  (Yes, this is an exceedingly minor issue.  Sorry.)
> +static HANDLE import_utf16_to_unicodetext(CFDataRef data)
> +{
> - const WCHAR *src;
> - unsigned long data_len;
> - unsigned long new_lines = 0;
> - LPWSTR dst;
> - unsigned long i, j;
> - HANDLE unicode_handle = NULL;

This is an unnecessary initialization / dead store, which is frowned upon.

Thanks!

-Ken

[PATCH] winemac.drv: Support the public UTF-16 type for Unicode text.

[Charles Davis]

On Aug 21, 2013, at 10:34 PM, Ken Thomases wrote:
> On Aug 21, 2013, at 9:42 PM, Charles Davis wrote:
> > +static HANDLE import_utf16_to_unicodetext(CFDataRef data)
> > +{
> > - const WCHAR *src;
> > - unsigned long data_len;
> > - unsigned long new_lines = 0;
> > - LPWSTR dst;
> > - unsigned long i, j;
> > - HANDLE unicode_handle = NULL;
>
> This is an unnecessary initialization / dead store, which is frowned upon.

I was only blindly copying the code that you wrote ;).

Chip

[PATCH] winemac.drv: Support the public UTF-16 type for Unicode text.

[Ken Thomases]

On Aug 21, 2013, at 11:49 PM, Charles Davis wrote:
> On Aug 21, 2013, at 10:34 PM, Ken Thomases wrote:
> > On Aug 21, 2013, at 9:42 PM, Charles Davis wrote:
> > > +static HANDLE import_utf16_to_unicodetext(CFDataRef data)
> > > +{
> > > - const WCHAR *src;
> > > - unsigned long data_len;
> > > - unsigned long new_lines = 0;
> > > - LPWSTR dst;
> > > - unsigned long i, j;
> > > - HANDLE unicode_handle = NULL;
> >
> > This is an unnecessary initialization / dead store, which is frowned upon.
>
> I was only blindly copying the code that you wrote ;).

Yeah, I know where it came from, but you then changed the code enough to make the initialization unnecessary where it had been necessary in the original. :)

-Ken