Re: (5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Dmitry Timoshkov]

"Tony Lambregts"  wrote:
> -IMPORTS   = kernel32 ntdll
> +IMPORTS   = kernel32 ntdll user32

I'm not sure Alexandre will like it. Until for now, creation of the circular
dependencies were prohibitted. Probably it's better to use swprintf from ntdll,
or a set of calls strcpyW/ntdll._ultow/strcatW.
> -LONG WINAPI RegSaveKeyW( HKEY hkey, LPCWSTR file, LPSECURITY_ATTRIBUTES sa )
> +LONG WINAPI RegSaveKeyA( HKEY hkey, LPCSTR file, LPSECURITY_ATTRIBUTES sa )
>  {
> - LPSTR fileA = HEAP_strdupWtoA( GetProcessHeap(), 0, file );
> - DWORD ret = RegSaveKeyA( hkey, fileA, sa );
> - if (fileA) HeapFree( GetProcessHeap(), 0, fileA );
> - return ret;
> -
> - UNICODE_STRING *fileW = &NtCurrentTeb()->StaticUnicodeString;
> - STRING fileA;
> -
> - if (!file || !*file)
> - {
> -
>    SetLastError(ERROR_PATH_NOT_FOUND);
> -
>    return FALSE;

As Alexandre has already pointed out, registry APIs are supposed to not change
last error value.
> - }
> -
> - RtlInitAnsiString(&fileA, file);
> - RtlAnsiStringToUnicodeString(fileW, &fileA, FALSE);
> - if (fileW) return RegSaveKeyW(hkey, fileW->Buffer, sa);
> - return FALSE;

fileW is always not NULL, so, there is no need to check it. What you really
need to check is a return value of RtlAnsiStringToUnicodeString.

(5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Tony Lambregts]

Dmitry Timoshkov wrote:
> "Tony Lambregts"  wrote:
> > -IMPORTS   = kernel32 ntdll
> > +IMPORTS   = kernel32 ntdll user32
>
> I'm not sure Alexandre will like it. Until for now, creation of the circular
> dependencies were prohibitted. Probably it's better to use swprintf from ntdll,

&*#@ Well. I am very much at the end of my rope here. As I said from the 
begining I could not find a way to use swprintf. If there is a way it is 
not obvious. Now I may be thick at times but IMO there should be a way 
of using swprintf here. So what am I missing.
> or a set of calls strcpyW/ntdll._ultow/strcatW.

I would prefer not to use this workaround except as a last resort. At 
least,  I would like some definitive reason why we can't use swprintf.
> > -LONG WINAPI RegSaveKeyW( HKEY hkey, LPCWSTR file, LPSECURITY_ATTRIBUTES sa )
> > +LONG WINAPI RegSaveKeyA( HKEY hkey, LPCSTR file, LPSECURITY_ATTRIBUTES sa )
> > {
> > - LPSTR fileA = HEAP_strdupWtoA( GetProcessHeap(), 0, file );
> > - DWORD ret = RegSaveKeyA( hkey, fileA, sa );
> > - if (fileA) HeapFree( GetProcessHeap(), 0, fileA );
> > - return ret;
> > -
> > - UNICODE_STRING *fileW = &NtCurrentTeb()->StaticUnicodeString;
> > - STRING fileA;
> > -
> > - if (!file || !*file)
> > - {
> > -
> >    SetLastError(ERROR_PATH_NOT_FOUND);
> > -
> >    return FALSE;
>
> As Alexandre has already pointed out, registry APIs are supposed to not change
> last error value.

OK.
> > - }
> > -
> > - RtlInitAnsiString(&fileA, file);
> > - RtlAnsiStringToUnicodeString(fileW, &fileA, FALSE);
> > - if (fileW) return RegSaveKeyW(hkey, fileW->Buffer, sa);
> > - return FALSE;
>
> fileW is always not NULL, so, there is no need to check it. What you really
> need to check is a return value of RtlAnsiStringToUnicodeString.

so this would be better.

-LONG WINAPI RegSaveKeyW( HKEY hkey, LPCWSTR file, LPSECURITY_ATTRIBUTES 
sa )
+LONG WINAPI RegSaveKeyA( HKEY hkey, LPCSTR file, LPSECURITY_ATTRIBUTES sa )
 {
-    LPSTR fileA = HEAP_strdupWtoA( GetProcessHeap(), 0, file );
-    DWORD ret = RegSaveKeyA( hkey, fileA, sa );
-    if (fileA) HeapFree( GetProcessHeap(), 0, fileA );
-    return ret;
+
+    UNICODE_STRING *fileW = &NtCurrentTeb()->StaticUnicodeString;
+    STRING fileA;
+   
+    if (!file || !*file)
+    {
+        return FALSE;
+    }
+
+    RtlInitAnsiString(&fileA, file);
+    if (!RtlAnsiStringToUnicodeString(fileW, &fileA, FALSE));
+    {
+        return RegSaveKeyW(hkey, fileW->Buffer, sa);
+    }
+    return FALSE;
+
 }

(5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Dmitry Timoshkov]

"Tony Lambregts"  wrote:
> > I'm not sure Alexandre will like it. Until for now, creation of the circular
> > dependencies were prohibitted. Probably it's better to use swprintf from ntdll,
>
> &*#@ Well. I am very much at the end of my rope here. As I said from the 
> begining I could not find a way to use swprintf. If there is a way it is 
> not obvious. Now I may be thick at times but IMO there should be a way 
> of using swprintf here. So what am I missing.

There should be a way of using at least exported unicode APIs from ntdll,
but currently there is no even a header with their function prototypes.
Probably a better way would be to add wrappers for ntdll unicode APIs to
wine_unicode and use them instead. Alexandre, what way would you prefer?
Or perhaps some other one?
> > or a set of calls strcpyW/ntdll._ultow/strcatW.
>
> I would prefer not to use this workaround except as a last resort. At 
> least,  I would like some definitive reason why we can't use swprintf.

Agreed.
> so this would be better.

[patch skipped]

Looks much better now. Thanks for your efforts.

(5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Alexandre Julliard]

"Dmitry Timoshkov"  writes:
> There should be a way of using at least exported unicode APIs from ntdll,
> but currently there is no even a header with their function prototypes.
> Probably a better way would be to add wrappers for ntdll unicode APIs to
> wine_unicode and use them instead. Alexandre, what way would you prefer?

We could certainly have a Unicode printf in libwine_unicode. Most of
the other functions should already have an equivalent AFAICS.

(5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Tony Lambregts]

Alexandre Julliard wrote:
> "Dmitry Timoshkov"  writes:
> > There should be a way of using at least exported unicode APIs from ntdll,
> > but currently there is no even a header with their function prototypes.
> > Probably a better way would be to add wrappers for ntdll unicode APIs to
> > wine_unicode and use them instead. Alexandre, what way would you prefer?
>
> We could certainly have a Unicode printf in libwine_unicode. Most of
> the other functions should already have an equivalent AFAICS.

So is this what you had in mind? (cvs is not working for me right now)

--- clean/include/wine/unicode.h	Thu Jan  2 12:25:44 2003
+++ wine/include/wine/unicode.h	Tue Mar 11 13:15:26 2003
@@ -76,6 +76,7 @@
  extern WCHAR *strstrW( const WCHAR *str, const WCHAR *sub );
  extern long int strtolW( const WCHAR *nptr, WCHAR **endptr, int base );
  extern unsigned long int strtoulW( const WCHAR *nptr, WCHAR **endptr, int base );
+extern int swprintf(WCHAR *str, const WCHAR *format, ...);

static inline int is_dbcs_leadbyte( const union cptable *table, unsigned char ch )
  {

(5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Dmitry Timoshkov]

"Alexandre Julliard"  wrote:
> We could certainly have a Unicode printf in libwine_unicode. Most of
> the other functions should already have an equivalent AFAICS.

Is it acceptable to simply copy the whole NTDLL_vsnwprintf to libwine_unicode?

(5th) Janitorial dlls/advapi32/registry.c W->A cleanup

[Alexandre Julliard]

"Dmitry Timoshkov"  writes:
> Is it acceptable to simply copy the whole NTDLL_vsnwprintf to libwine_unicode?

Well it should of course be a move, not a copy; but yes that's fine.