It seems that valgrind doesn't like Ubuntu 7.10; I had to drop back to my Feisty system to generate good valgrind stack dumps. And the tests didn't hang once!
Results for the last two days are at http://kegel.com/wine/valgrind/20071112/ http://kegel.com/wine/valgrind/20071113/
The logs are trimmed a bit more to make diffs less noisy. Diff now shows clearly that there were significant improvements in three tests between those two runs: ole32/compobj, ole32/marshal, and user32/dde Errors still remain in all three tests, though. - Dan
"Dan Kegel" dank@kegel.com wrote:
It seems that valgrind doesn't like Ubuntu 7.10; I had to drop back to my Feisty system to generate good valgrind stack dumps. And the tests didn't hang once!
Results for the last two days are at http://kegel.com/wine/valgrind/20071112/ http://kegel.com/wine/valgrind/20071113/
While looking at the valgrind reports above I noticed that a lot of warnings are triggered in NTDLL_queue_process_apc by the fact that not the whole apc_call_t union is initialized before passing it to the server. In contrast SERVER_START_REQ always initializes __server_request_info to 0 before the client starts to fill the fields. Probably that's the question to Alexandre whether we should memset(0) apc_call_t before filling it, or just ignore valgrind warnings.
On Nov 14, 2007 10:16 PM, Dmitry Timoshkov
While looking at the valgrind reports above I noticed that a lot of warnings are triggered in NTDLL_queue_process_apc by the fact that not the whole apc_call_t union is initialized before passing it to the server. In contrast SERVER_START_REQ always initializes __server_request_info to 0 before the client starts to fill the fields. Probably that's the question to Alexandre whether we should memset(0) apc_call_t before filling it, or just ignore valgrind warnings.
Oh, he'd undoubtedly prefer ignoring to memsetting. But he would like it even better if we can avoid sending unneeded bytes. Are the extra bytes at the end (they ought to be)? If so, whatever decides how many bytes to send could be just a bit smarter. - Dan
p.s. the mailing list archives seem to be not archiving new messages :-(
"Dan Kegel" dank@kegel.com wrote:
On Nov 14, 2007 10:16 PM, Dmitry Timoshkov
While looking at the valgrind reports above I noticed that a lot of warnings are triggered in NTDLL_queue_process_apc by the fact that not the whole apc_call_t union is initialized before passing it to the server. In contrast SERVER_START_REQ always initializes __server_request_info to 0 before the client starts to fill the fields. Probably that's the question to Alexandre whether we should memset(0) apc_call_t before filling it, or just ignore valgrind warnings.
Oh, he'd undoubtedly prefer ignoring to memsetting. But he would like it even better if we can avoid sending unneeded bytes. Are the extra bytes at the end (they ought to be)? If so, whatever decides how many bytes to send could be just a bit smarter.
Since wineserver sequests are sent without an attempt to reduce the size of written data (and memset(0) is a part of SERVER_START_REQ macro) I'd assume that it's ok to do the same for server APC requests. Otherwise server calls need to be optimized as well.
On Nov 15, 2007 2:33 AM, Dmitry Timoshkov dmitry@codeweavers.com wrote:
Since wineserver sequests are sent without an attempt to reduce the size of written data (and memset(0) is a part of SERVER_START_REQ macro) I'd assume that it's ok to do the same for server APC requests. Otherwise server calls need to be optimized as well.
I did optimize one recently for just this reason: http://www.winehq.org/pipermail/wine-cvs/2007-October/037608.html
I haven't looked to see if the same technique can be used in the NTDLL_queue_process_apc case.
"Dan Kegel" dank@kegel.com writes:
Oh, he'd undoubtedly prefer ignoring to memsetting. But he would like it even better if we can avoid sending unneeded bytes. Are the extra bytes at the end (they ought to be)? If so, whatever decides how many bytes to send could be just a bit smarter.
Actually memset is OK for server requests, not because of Valgrind but because of backwards compatibility concerns. It makes it possible to extend a request knowing that old clients will send 0 for the new fields.
p.s. the mailing list archives seem to be not archiving new messages :-(
Jer restarted mailman and it seems to be fixed now. Presumably a side effect of the db going nuts yesterday.
cheers,
Jeremy
Dan Kegel wrote:
Oh, he'd undoubtedly prefer ignoring to memsetting.
I believe the "official" answer is to teach valgrind which fields are important for which server request. Granted, it a lot more work, but it's the only way we will actually catch errors :-)
Shachar
-
Alexandre Julliard -
Dan Kegel -
Dmitry Timoshkov -
Jeremy White -
Shachar Shemesh