"Jaco Greeff" jaco@puxedo.org writes:
I'm not sure if I should send the whole patch or just do increments for the new stuff. At this point it is everything, including cleanups of the test case and use of correct WideCharToMultiByte as suggested by Dimitri. In additions the configure.ac patch for the dlls/msvcrt/tests/Makefile.in is added.
As always, comments are very welcome to get this thing bedded down finally.
- you can't use the libc printf to print unicode chars, the format is not the same - WideCharToMultiByte can output several characters for a single unicode char, you need to take that into account for buffer sizes - you cannot cast a WCHAR* into a CHAR* to convert the format to ASCII - there's no point in using strncat if you don't pass it the real size of the buffer - please don't copy code from MSDN, it is copyrighted by Microsoft.
Alexandre Julliard wrote:
- you can't use the libc printf to print unicode chars, the format is not the same
Umm, I'm not and if I am, I should be shot. The logic behind what I'm doing is more or less as follows:
1. Print all non-formatting characters to file; 2. Parse the formatting string, converting between %S and %s. 3. Print the formatting string with arguments, converting the unicode strings via WideStringToMultiByte;
So yes, the strings will be converted before being printed since I'm printing them via the %s formatting specifier. (And the test code results does show that this is indeed the case)
- WideCharToMultiByte can output several characters for a single unicode char, you need to take that into account for buffer sizes
*grumble* Thanks. I'm now making now that I get the length of the result string via WideCharToMultiByte before allocating and filling the buffer with another call to WideCharToString. Initially I thought that a strlenW would suffice, obviously not.
- you cannot cast a WCHAR* into a CHAR* to convert the format to ASCII
You had me stumped here for a while as to where I'm doing this stupidity. A search brought this up:
case (WCHAR)L'd': case (WCHAR)L'i': case (WCHAR)L'o': case (WCHAR)L'u': case (WCHAR)L'X': case (WCHAR)L'x': pFormat->nType = VFMT_INTEGER; strncat(pFormat->szFmt, (CHAR *)szFmt, 1); break; ...
I want to grab the last formatting char and append it to my buffer. Now I could do,
... case (WCHAR)L'u': pFormat->nType = VFMT_INTEGER; strcat(pFormat->szFmt, "u"); break; case (WCHAR)L'X': pFormat->nType = VFMT_INTEGER; strcat(pFormat->szFmt, "X"); break; case (WCHAR)L'x': pFormat->nType = VFMT_INTEGER; strcat(pFormat->szFmt, "x"); break; ...
for every instance, which problably is more readable/understandable (and the way I had it before I "optimized" a bit :)). What I did put down in the first instance however, does work as intended since I'm only interrested in the first byte/character pointed to by the szFmt pointer. If "sizeof(CHAR) != 1" and "arch != ix86" then we are in trouble here. However, you are right, it is problably confusing and will make maintenance a bitch.
- there's no point in using strncat if you don't pass it the real size of the buffer
As I'm building up the format string, I'm using it to append only the current formatting character to the buffer I'll eventually be using to print. A suggestion for a better approach is always welcome.
- please don't copy code from MSDN, it is copyrighted by Microsoft.
I won't include the test case in the next patch. As a real wine test case I'm anyway not sure of the full value, as a debugging tool and making sure that I get the correct output, it is extremely valuable.
Greetings, Jaco
Jaco Greeff jaco@puxedo.org writes:
Umm, I'm not and if I am, I should be shot. The logic behind what I'm doing is more or less as follows:
- Print all non-formatting characters to file;
- Parse the formatting string, converting between %S and %s.
- Print the formatting string with arguments, converting the unicode
strings via WideStringToMultiByte;
So yes, the strings will be converted before being printed since I'm printing them via the %s formatting specifier. (And the test code results does show that this is indeed the case)
You were doing it for strings but not for characters; but I see you have fixed this now. Still, this only works for vfprintf, vfwprintf has to output in Unicode so you cannot convert to ASCII or use the system printf for that. It would probably be easier to grab a printf implementation from somewhere and convert it to Unicode.
- there's no point in using strncat if you don't pass it the real size of the buffer
Also be aware that strncpy/strncat are required to zero out the unfilled part of the buffer. This is a performance penalty you don't (normmaly) want. They also don't guarantee to null terminate the string.
Some systems have strlcpy/strlcat which DTRT.
David
David Laight wrote:
Also be aware that strncpy/strncat are required to zero out the unfilled part of the buffer. This is a performance penalty you don't (normmaly) want.
Yes, and we don't really want the performance penalty in this case. In my last patch I've refrained from using strncat(x, y, 1), rather using strcat(x, y). There are other ways of doing this, i.e. strcat in this case might not be the best/fastest, my first priority is getting the stuff acceptable to all and into cvs and then making incremental adjustments to squeeze the most out of it.
Greetings, Jaco
-
Alexandre Julliard -
David Laight -
Jaco Greeff