On Tue, 2005-01-04 at 12:21 -0600, Robert Shearman wrote:
I can't see the need to make this non-static at the moment.
Right, I made it non-static when the lock was taken inside an inlined function earlier, but I got rid of that at the end. It can go back to being static.
I don't really like functions like this doing special things on certain input values. I don't think it is that big a deal to require the few callers of this function to go to the effort of getting the apartment themselves.
It was done that way so it's inside the lock, ie the TEB apt field should be cleared before the apartment is freed, otherwise there's a time when it's pointing to garbage.
But thinking about it, as that's TLS it probably doesn't matter. So I'll remove this feature.
I think it would be cleaner to put the ref-count increment inside the apartment creation function. That way, no callers of the COM_CreateApartment function can make the mistake of creating an apartment without incrementing its refcount.
OK, done.
HRESULT WINAPI CoGetState(IUnknown ** ppv) {
- APARTMENT * apt = COM_CurrentInfo();
- HRESULT hr = E_FAIL;
- FIXME("\n");
- FIXME("possible stub\n");
I believe the function does everything that is required of it - the only problem is that we don't currently do anything with the state. I'm not sure if we should though.
Do we actually have any docs on this function? I agree the "possible stub" fixme is annoying though so I'll remove it.
This should be moved into the TLS struct, not removed entirely.
inits has been subsumed into the refcount, each CoInitialize adds a ref. Why is it still needed?
LPVOID ErrorInfo; /* thread error info */
You can remove this - it should no longer be needed.
Right, done.
- if (!NtCurrentTeb()->ReservedForOle)
NtCurrentTeb()->ReservedForOle = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct oletls));I guess are ignoring failures from HeapAlloc then. There isn't really much we can do about it anyway though.
Indeed, we're a bit hosed in this situation. I guess the relevant parts of the code should return an appropriate error code for that specific API though.
Nothing should reference it directly as the offset is OS dependent.
Hehe, whatever :) I've seen too many "neat tricks" based on referencing stuff at that offset. Still I took the comment out as we don't know the format of that object anyway, so it's a bit useless ....
Here's the updated patch.
Mike Hearn mh@codeweavers.com - Make apartment access thread-safe by introducing refcounting and wider usage of the apartment lock - Rework OLE TLS management to eliminate uninitialised apartments and parent chaining