Re: [2/3] ntdll: fill unused tails of heap blocks if HEAP_FREE_CHECKING_ENABLED (take 3)
Dan Kegel dank@kegel.com writes:
To check whether we're too early in process setup to check global flags, I compare the arguments of RtlCreateHeap with the ones used by the call in virtual_init(). This is not especially clean, but I couldn't think of anything better. I can hear Alexandre hitting the 'd' key already, so if anyone has a better idea, please let me know.
That's a sure hint that you shouldn't have submitted the patch. As I told you on IRC, you have to make the patch the best you possibly can *before* showing it to me. That's doubly true for a patch that has already seen several rounds. If you truly can't find a better way you can always ask for suggestions on IRC or wine-devel. Submitting broken patches (worse, that you already know are broken) is only going to make it harder for you to get anything in.
On Fri, Jan 8, 2010 at 2:08 AM, Alexandre Julliard julliard@winehq.org wrote:
To check whether we're too early in process setup to check global flags, I compare the arguments of RtlCreateHeap with the ones used by the call in virtual_init(). This is not especially clean, but I couldn't think of anything better. I can hear Alexandre hitting the 'd' key already, so if anyone has a better idea, please let me know.
That's a sure hint that you shouldn't have submitted the patch. As I told you on IRC, you have to make the patch the best you possibly can *before* showing it to me. That's doubly true for a patch that has already seen several rounds. If you truly can't find a better way you can always ask for suggestions on IRC or wine-devel. Submitting broken patches (worse, that you already know are broken) is only going to make it harder for you to get anything in.
Posting a working if suboptimal implementation is a clear way of framing questions sometimes, but I guess when I do that it should be to wine-devel.
Maybe what we want is a safe implementation of RtlGetNtGlobalFlags(), one that returns zero if it's not safe to call NtCurrentTeb(). We could store the global flags in a plain old global variable, and copy it in to the peb but never reference it there, I suppose. Would that be clean enough for you? Or is that still too kludgy? - Dan
On Fri, Jan 8, 2010 at 6:41 AM, Dan Kegel dank@kegel.com wrote:
Maybe what we want is a safe implementation of RtlGetNtGlobalFlags(), one that returns zero if it's not safe to call NtCurrentTeb(). We could store the global flags in a plain old global variable, and copy it in to the peb but never reference it there, I suppose. Would that be clean enough for you? Or is that still too kludgy?
Or would you prefer Joris' approach:
couldn't you place the RtlCreateHeap implementation with an internal function, that takes an extra argument (boolean or flags) to indicate whether to do this initialization step or not ? Then, call the internal function from virtual_init() with the boolean set, and have RtlCreateHeap just calling it, without the boolean set. I've seen something like this being done elsewhere (but for different reasons)
?
Dan Kegel dank@kegel.com writes:
Posting a working if suboptimal implementation is a clear way of framing questions sometimes, but I guess when I do that it should be to wine-devel.
Questions and RFCs should go to wine-devel. Sending to wine-patches means "I believe this is good enough to be committed".
Maybe what we want is a safe implementation of RtlGetNtGlobalFlags(), one that returns zero if it's not safe to call NtCurrentTeb(). We could store the global flags in a plain old global variable, and copy it in to the peb but never reference it there, I suppose. Would that be clean enough for you? Or is that still too kludgy?
There's no reason that RtlGetNtGlobalFlags would need the TEB at all.
On Fri, Jan 8, 2010 at 6:52 AM, Alexandre Julliard julliard@winehq.org wrote:
There's no reason that RtlGetNtGlobalFlags would need the TEB at all.
Well, that's where native puts the flags. OK, I'll put them elsewhere.
-
Alexandre Julliard -
Dan Kegel