On 2011-03-15 22:09, Stefan Küng wrote:
> On Tue, Mar 15, 2011 at 21:54, Oto BREZINA<otik_at_printflow.eu> wrote:
>
>> This also shifts Invalid handle value to NULL. What make me think if
>> patch r20974 was made in right way.
[...]
>>> So IMHO while the idea of a wrapper class is good, it is dangerous.
>>> I'd like to keep the code as is, using the handles for each API
>>> separately according to its documentation.
>> I would say not using it is dangerous, using class built without
>> understanding is dangerous even more. That's one of most important
>> reasons I send as the conceptual patch, not just a patch. Something to
>> check.
>>
>> It's your code and you are still main developer - you would be affected
>> mostly; I just tried to share my thoughts...
> Using a wrapper class is dangerous here, because once we start using
> it, people tend to get lazy (at least I do) and forget to check and
> make sure the class can really be used. For most situations, it would
> work great. But then comes the situation where the API actually allows
> INVALID_HANDLE_VALUE as a pseudo handle value and since we're so used
> to using the wrapper, we simply don't check and then it will fail.
> Definitely not while we're debugging it but later on some users
> machine...
>
> That's why I don't like to use wrapper classes if there's no way to
> make them work in all situations, or break *reliably* in situations
> where it must not be used.
I still don't think that there is no way to make it working in all
situations, just not sure as worthy it is.
> And I think that's exactly the reason why the existing CHandle class
> only supports NULL as the invalid value and not INVALID_HANDLE_VALUE.
Sounds as good start point for me :)
Let say:
- Invalid is only NULL,
- INVALID_HANDLE_VALUE - don't call CloseHandle
- You are forced to enter invalid value into constructor
CTsvnHandle::CTsvnHandle(HANDLE h, HANDLE hInvalid) ....
m_h = (h==hInvalid) ? NULL : h;
BIG BAD new invalid value is NULL while you code always checks for
INVALID_HANDLE_VALUE.
Even this may be handled in operator= method
> Stefan
Oto
PS: It seems I'm going to fix poChecker it may be more worthy ...
--
Oto BREZINA, Printflow s.r.o., EU
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2711761
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2011-03-15 22:29:50 CET