"Markus Hitter" mah@jump-ing.de wrote:
Providing the file handle allows to map read/write requests to the corresponding file name.
As pointed out by Alexander, you can use an appropriate debug channel for that, +relay or +server. There is no need to pollute the source with additional traces.
Am 27.08.2008 um 10:14 schrieb Dmitry Timoshkov:
"Markus Hitter" mah@jump-ing.de wrote:
Providing the file handle allows to map read/write requests to the corresponding file name.
As pointed out by Alexander, you can use an appropriate debug channel for that, +relay or +server. There is no need to pollute the source with additional traces.
If you think this way, you should get rid of all the TRACE()s in this source file.
For me, it makes a difference, wether I can focus on file operations or if I have to wade through gigabyte-sized (no joke) log files. To give you an example how it looks:
trace:ntdll:NtCreateFile handle=0x339750 access=80000000 name=L"\??\ \C:\Programme\Dassault Systemes\B16\intel_a\EnvName.txt" objattr=00000042 root=(nil) sec=(nil) io=0x339724 alloc_size=(nil) attr=00000080 sharing=00000003 disp=1 options=00000050 ea=(nil). 0x00000000 trace:ntdll:NtCreateFile returning handle 0x144 trace:ntdll:NtReadFile (0x144,(nil),(nil),(nil), 0x3397c4,0xd93888,0x00001000,(nil),(nil)),partial stub! trace:ntdll:NtReadFile = SUCCESS (17)
All but one of these lines are already there.
As there's nothing left to simplify, I take this patch back.
MarKus
- - - - - - - - - - - - - - - - - - - Dipl. Ing. Markus Hitter http://www.jump-ing.de/
"Markus Hitter" mah@jump-ing.de wrote:
If you think this way, you should get rid of all the TRACE()s in this source file.
For me, it makes a difference, wether I can focus on file operations or if I have to wade through gigabyte-sized (no joke) log files.
We all are in the same boat. If you need some additional traces for debugging purposes you can always add them temporarily.
Am 27.08.2008 um 11:40 schrieb Dmitry Timoshkov:
We all are in the same boat.
Next time, please try to speak up earlier and more clearly. Four reviews, three patch reworks were done and about 20 messages were written, just to find out _any_ change is not welcome.
MarKus
- - - - - - - - - - - - - - - - - - - Dipl. Ing. Markus Hitter http://www.jump-ing.de/
"Markus Hitter" mah@jump-ing.de wrote:
We all are in the same boat.
It seems you didn't get the message.
Next time, please try to speak up earlier and more clearly. Four reviews, three patch reworks were done and about 20 messages were written, just to find out _any_ change is not welcome.
If you would follow the wine-patches/wine-devel/wine-cvs mailing lists for awhile you'd find out that changes *are* welcome, not any, no, the change should be useful at least.
Am Mittwoch, den 27.08.2008, 17:14 +0900 schrieb Dmitry Timoshkov:
"Markus Hitter" mah@jump-ing.de wrote:
Providing the file handle allows to map read/write requests to the corresponding file name.
As pointed out by Alexander, you can use an appropriate debug channel for that, +relay or +server. There is no need to pollute the source with additional traces.
I am sorry, but I don't get your point. The handle is returned in an out-pointer. There is no way to get that info into +relay. Alexandre said:
| Module-specific traces like this one should try to print as | little information as necessary to make sense of the trace | In general a single trace at function entry is enough to understand | the code flow. I agree, but the handles returned *are* necessary to make sense of the trace. Also, if I just want parameters at the function entry: This information is provided by +relay. In the +file log, I don't care about the address where the handle should be written to, yet it is logged by our tracing conventions. It also doesn't help to understand code flow. Now we are rejecting a patch that logs useful information instead, which is vital to connect further file calls to the NtCreateCall, i.e. understand the flow. I don't understand that.
| If you want details like the exact error codes you can use +relay or | +server. I agree too, but the details about exact error codes have been removed from the patch.
Regards, Michael Karcher
[The original mail seemed to be stuck in the moderation queue or got lost somewhere else. Reposting with correct "From:" this time]
Am Mittwoch, den 27.08.2008, 17:14 +0900 schrieb Dmitry Timoshkov:
"Markus Hitter" mah@jump-ing.de wrote:
Providing the file handle allows to map read/write requests to the corresponding file name.
As pointed out by Alexander, you can use an appropriate debug channel for that, +relay or +server. There is no need to pollute the source with additional traces.
I am sorry, but I don't get your point. The handle is returned in an out-pointer. There is no way to get that info into +relay. Alexandre said:
| Module-specific traces like this one should try to print as | little information as necessary to make sense of the trace | In general a single trace at function entry is enough to understand | the code flow. I agree, but the handles returned *are* necessary to make sense of the trace. Also, if I just want parameters at the function entry: This information is provided by +relay. In the +file log, I don't care about the address where the handle should be written to, yet it is logged by our tracing conventions. It also doesn't help to understand code flow. Now we are rejecting a patch that logs useful information instead, which is vital to connect further file calls to the NtCreateCall, i.e. understand the flow. I don't understand that.
| If you want details like the exact error codes you can use +relay or | +server. I agree too, but the details about exact error codes have been removed from the patch.
Regards, Michael Karcher
Michael Karcher wrote:
[The original mail seemed to be stuck in the moderation queue or got lost somewhere else. Reposting with correct "From:" this time]
Am Mittwoch, den 27.08.2008, 17:14 +0900 schrieb Dmitry Timoshkov:
"Markus Hitter" mah@jump-ing.de wrote:
Providing the file handle allows to map read/write requests to the corresponding file name.
As pointed out by Alexander, you can use an appropriate debug channel for that, +relay or +server. There is no need to pollute the source with additional traces.
I am sorry, but I don't get your point. The handle is returned in an out-pointer. There is no way to get that info into +relay. Alexandre said:
| Module-specific traces like this one should try to print as | little information as necessary to make sense of the trace | In general a single trace at function entry is enough to understand | the code flow. I agree, but the handles returned *are* necessary to make sense of the trace. Also, if I just want parameters at the function entry: This information is provided by +relay. In the +file log, I don't care about the address where the handle should be written to, yet it is logged by our tracing conventions. It also doesn't help to understand code flow. Now we are rejecting a patch that logs useful information instead, which is vital to connect further file calls to the NtCreateCall, i.e. understand the flow. I don't understand that.
| If you want details like the exact error codes you can use +relay or | +server. I agree too, but the details about exact error codes have been removed from the patch.
Almost no programs are calling the Nt* functions directly. But kernel32 functions. So the point about knowing the file handle in return from NtCreateFile is moot. And kernel32's CreateFile() already prints that information for you.
This is why you are getting so much resistance - you trying to add something redundant.
Am Samstag, den 30.08.2008, 11:41 -0600 schrieb Vitaliy Margolen:
Almost no programs are calling the Nt* functions directly. But kernel32 functions. So the point about knowing the file handle in return from NtCreateFile is moot. And kernel32's CreateFile() already prints that information for you.
This is not true anymore for kernel level device drivers wine starts to support now. ntoskrnl.exe:NtCreateFile is directly forwarded into ntdll.
This is why you are getting so much resistance - you trying to add something redundant.
I think this is not a valid point.
First, most of the function entry traces also are "redundant", as a relay trace already prints the parameters. Relay tracing can be limited to certain dlls, for example ntdll. In that case, most of the traces in file.c are redundant.
Second, kernel32:CreateFile logs to file, whereas ntdll:NtCreateFile logs to ntdll. So to make sense of these details in the ntdll trace, you have to turn on file. As this is dealing with files, it is not too ugly, but keeping information needed to follow code flow in one channel is a definite pro.
Third, there is precedence in NtReadFile[Scatter] and NtWriteFile[Gather]. Would you suggenst to remove the result traces from there?
Regards, Michael Karcher
-
Dmitry Timoshkov -
Markus Hitter -
Michael Karcher -
Michael Karcher -
Vitaliy Margolen