Re: AW: Conceptual Patch - Handle wrapper class

On 2011-03-15 20:19, Stefan Küng wrote:
> On 15.03.2011 10:48, Oto BREZINA wrote:
> I doubt that Raymond Chen is ever wrong when it comes to the Windows
> API :)
> But the blog post indicates that while INVALID_HANDLE_VALUE has its
> origins in win16, even new APIs use it: "[...]whenever a new function
> got added, it was a bit of a toss-up whether the new function returned
> NULL or INVALID_HANDLE_VALUE."
Sorry, I misunderstand this. I stopped on first sentence. :o
>> Can be NULL valid? Can be INVALID_HANDLE_VALUE valid?
> From the blog post you linked before:
> http://blogs.msdn.com/b/oldnewthing/archive/2004/03/02/82639.aspx
>
> "Fourth, you have to be particularly careful with the
> INVALID_HANDLE_VALUE value: By coincidence, the value
> INVALID_HANDLE_VALUE happens to be numerically equal to the
> pseudohandle returned by GetCurrentProcess(). Many kernel functions
> accept pseudohandles [...]"
>
> So INVALID_HANDLE_VALUE *can* be a valid handle (a pseudo handle, but
> still a valid one). And "many kernel functions accept pseudohandles"
> indicates that this is not just one API but several.
>
I think I mention this a bit in first email.
Quote "As I understand INVALID_HANDLER_VALUE is also pseudo handler to
windows itself, or something like that."
Ok, INVALID_HANDLE_VALUE CAN be valid value for handle, but can not be
closed - I would say one point for Helper Class.

This also shifts Invalid handle value to NULL. What make me think if
patch r20974 was made in right way.

For m_hPipe is errorous INVALID_HANDLE_VALUE, while for m_hEvent it is NULL.

So ClosePipe should looks like:

void CRemoteCacheLink::ClosePipe()
{
     AutoLocker lock(m_critSec);

     if(m_hPipe != INVALID_HANDLE_VALUE)
     {
         CloseHandle(m_hPipe);
         m_hPipe = INVALID_HANDLE_VALUE;
     }
     if(m_hEvent != NULL)
     {
         CloseHandle(m_hEvent);
         m_hEvent = NULL;
     }
}

Is this easy readable and maintainable ?

> 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...

> Stefan
Oto

-- 
Oto BREZINA, Printflow s.r.o., EU
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2711753
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].