Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
- 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: [EMAIL PROTECTED]
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
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
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
Jaco Greeff [EMAIL PROTECTED] 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: 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) 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. -- Alexandre Julliard [EMAIL PROTECTED]
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
On 29 Oct 2002 10:53:32 -0800, Alexandre Julliard [EMAIL PROTECTED] wrote : 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. Can I swear on this mailing list? Please, I need to. And one would think I've never used the vfwprintf function before the way I'm making all these wrong assumptions. Damnit, I know the Win32 API. *sigh* Luckily my wife got most of the frustrations at my stupidity... On your suggestion: I've looked at the glibc one. No comments. I'll refrain and look a bit more... I do feel that we are making progress however, albeit slowly ;) Do you think it a good idea just doing the ASCII version now and get it into CVS without the W version. (Ripping out the vfwprintf for now.) I can then proceed to make this ASCII implementation into a full real-prntf implementation and subsequently, when we are all happy do the Unicode conversion. Basically I'm suggesting breaking it up into small(er) steps. Comments? Greetings, Jaco
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
On Tue, Oct 29, 2002 at 07:36:06PM -, Jaco Greeff wrote: On 29 Oct 2002 10:53:32 -0800, Alexandre Julliard [EMAIL PROTECTED] wrote : 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. Can I swear on this mailing list? Please, I need to. And one would think I've never used the vfwprintf function before the way I'm making all these wrong assumptions. Damnit, I know the Win32 API. *sigh* Luckily my wife got most of the frustrations at my stupidity... [insert some rant about certain highly non-rewarding functions here...] Still, you're doing some pretty essential work, so let me just say thanks for your (annoying ?) work ! -- Andreas MohrStauferstr. 6, D-71272 Renningen, Germany Tel. +49 7159 800604http://mohr.de.tt
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
On Tue, 29 Oct 2002 21:10:52 +0100, Andreas Mohr [EMAIL PROTECTED] wrote : [insert some rant about certain highly non-rewarding functions here...] Still, you're doing some pretty essential work, so let me just say thanks for your (annoying ?) work ! Annoying? Sometimes. Boring? Never. I'm enjoying every minute of it, as long as I don't get to frustrate myself with idiotic mistakes. (Of which I've had a couple in the last week, the last one just took the prize :)) O, well, maybe I just need some extra sleep. Thanks a lot, encouragement is always appreciated.
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
Jaco Greeff wrote: On Tue, 29 Oct 2002 21:10:52 +0100, Andreas Mohr [EMAIL PROTECTED] wrote : [insert some rant about certain highly non-rewarding functions here...] Still, you're doing some pretty essential work, so let me just say thanks for your (annoying ?) work ! Annoying? Sometimes. Boring? Never. I'm enjoying every minute of it, as long as I don't get to frustrate myself with idiotic mistakes. (Of which I've had a couple in the last week, the last one just took the prize :)) I don't know. My personal best is to spend two weeks chasing a one wrong character bug. What's more, it was introduced after adding ~20 lines of code, and I knew it. Beat that! To my credit I will say that this was assembly (68000), and that the problem manifested itself in such an unrelated area of the program, that I overruled my own judgment and searched the problem in the vicinity of where it happened. As life would have it, the real problem was a missing # (which means use immediate value X, so instead it read Take value from address X). O, well, maybe I just need some extra sleep. Thanks a lot, encouragement is always appreciated.
Re: Bug #321: dlls/msvct/vf[w]printf functions
Jaco Greeff [EMAIL PROTECTED] wrote: Back to the real question: is there anything else missing that should be looked at in the patch? (Apart for the ugly test case and char conversion.) Doing a cursory look I don't see anything other (again, apart from Lsomething and (WCHAR)L'%'). -- Dmitry.
Re: Bug #321: dlls/msvct/vf[w]printf functions
Dmitry Timoshkov wrote:
Doing a cursory look I don't see anything other (again, apart from Lsomething
and (WCHAR)L'%').
That (the WCHARL'%' switch/case) is the one case for which I don't have
an answer for. Originally I had the code looking as follows:
INT somefunc(LPCWSTR lpszStr)
{
while (*lpszStr) {
switch (*lpszStr) {
case '%': do_something(); break;
...
In other parts of the wine code (I need to find it again, but a grep on
the sources should turn it up), generally these types of things gets
written as (maybe I just looked in entirely the wrong place ;),
INT somefunc(LPCWSTR lpszStr)
{
while (*lpszStr) {
switch (*lpszStr) {
case (WCHAR)L'%': do_something(); break;
...
and consequently I've adapted my code to do the same. I'm not sure what
is the correct approach to take in this regard. So, yes, I'm quite
happy to rid ourselves of the Lsomething in the test code and making
sure I use WideCharToMultiByte in the correct place, for the above case
I need some guidance.
What is the best way to handle these? If the (WCHAR)L'%' thing is not
the way to go, I'll be happy to make a patch for the other parts as
well. (Once I have an idea of the best way to handle this.)
Greetings,
Jaco
Re: Bug #321: dlls/msvct/vf[w]printf functions
As far as C is concerned, there can be no difference between case '%': do_something(); break; and case (WCHAR)L'%': do_something(); break; except that the latter is rather more complicated! Remember '%' is an int constant. David -- David Laight: [EMAIL PROTECTED]
Re: Bug #321: dlls/msvct/vf[w]printf functions
Jaco Greeff [EMAIL PROTECTED] wrote: What is the best way to handle these? If the (WCHAR)L'%' thing is not the way to go, I'll be happy to make a patch for the other parts as well. (Once I have an idea of the best way to handle this.) As there was already discussed many times and as David Laight pointed out, there is no need to make simple things look complicated. In your sample above, simple '%' will perfectly do the job. -- Dmitry.
Re: Bug #321: dlls/msvct/vf[w]printf functions
David Laight wrote: Remember '%' is an int constant. Agreed. The question of (WCHAR)L'%' vs. '%' is really one of semantics. Yes, the WCHAR approach looks more complicated, but is potentially better to understand the gist of what it supposed to happen and what is required. You know that you are working with WCHARs instead of CHARs, complicating things but potentially making it more understandable... Anyway, let me get back to fixing Dimitri's nigglies and stop worrying about semantics :)
Re: Bug #321: dlls/msvct/vf[w]printf functions (Patch 2)
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
Re: Bug #321: dlls/msvct/vf[w]printf functions
On 27 Oct 2002 22:28:33 -, Jaco Greeff [EMAIL PROTECTED] wrote : Changelog: - Test case for the printf functions to test all known cases of formatting (test case based on example found on MSDN site) The test case is mostly the same as this example with \t replaced with spaces and some extra \n characters: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt_printf.2c_.wprintf.asp Output from a test run attached to this mail. Greetings, Jaco printf.tst Description: Binary data
Re: Bug #321: dlls/msvct/vf[w]printf functions
Jaco Greeff [EMAIL PROTECTED] wrote:
Changelog:
* dlls/msvcrt/vfprintf.c, dlls/msvcrt/Makefile.in,
dlls/msvcrt/tests/printf.c, dlls/msvcrt/tests/Makefile.in: Jaco Greeff
[EMAIL PROTECTED]
- Full implementation to allow for %C %S in all printf related functions
- Test case for the printf functions to test all known cases of formatting
(test case based on example found on MSDN site)
I'm sorry for returning to that topic again and again, but it's really
does matter.
+static void test_printf(void)
+{
+ FILE* f;
+ char ch = 'a', *string = computer;
+ int count = -9234;
+ double fp = 251.7366;
+ wchar_t wch = L'w', *wstring = LUnicode;
+ void *p = 0x1234ABCD;
Please,
1. use explicit WCHAR type instead of wchar_t.
2. explicitly encode unicode strings.
wctomb(ch, wch);
3. directly use Win32 conversion APIs (WideCharToMultiByte in this case).
Take a look at other places in the Wine source how to do it properly.
Thanks.
--
Dmitry.
Re: Bug #321: dlls/msvct/vf[w]printf functions
Dmitry Timoshkov wrote: + wchar_t wch = L'w', *wstring = LUnicode; + void *p = 0x1234ABCD; 1. use explicit WCHAR type instead of wchar_t. 2. explicitly encode unicode strings. Agreed, that one is directly from the MS site (link attached to a previous mail) and I've used it as such as a test case. It is very ugly, and in addition you get millions of warnings just compiling it. I wanted to make sure that we get as close to a real-world example working hence not stuffing around (making it real-wine-code) too much. The point has been proven, real-world code works, I'll fix that up to be compilant. wctomb(ch, wch); 3. directly use Win32 conversion APIs (WideCharToMultiByte in this case). Damnit, must have missed that one. I'm sure I had WideCharToMultiByte before. Take a look at other places in the Wine source how to do it properly. I know how to do it properly, reasons for the irritations explained above. Back to the real question: is there anything else missing that should be looked at in the patch? (Apart for the ugly test case and char conversion.) Greetings, Jaco
