[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: AW: Conceptual Patch - Handle wrapper class

From: Oto BREZINA <otik_at_printflow.eu>
Date: Tue, 15 Mar 2011 22:27:38 +0100

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
Even this may be handled in operator= method

> Stefan

PS: It seems I'm going to fix poChecker it may be more worthy ...

Oto BREZINA, Printflow s.r.o., EU
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2011-03-15 22:29:50 CET

This is an archived mail posted to the TortoiseSVN Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.