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

2013-08-22 Thread 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

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

2013-08-22 Thread Ken Thomases 
On Aug 22, 2013, at 12:07 AM, Charles Davis wrote:

 Try 2: Eliminiate dead store. Sort export functions by name.

Looks OK to me.

-Ken

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

2013-08-21 Thread 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

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

2013-08-21 Thread 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