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

Re: Patch: Wrong ownership sematics for BSTRs

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: Fri, 15 Jan 2010 19:30:08 +0100

On 15.01.2010 08:16, Dmitry wrote:
> In this patch - one more occurence of BSTR leakage and wrong handling
> of IUnknown::QI() in class factories creation.
>
> What I find very confusing is checking the pointer returned by "new"
> for being null. See this SO question:
> http://stackoverflow.com/questions/550451/will-new-return-null-in-any-case
> Basically unless you take very special measures usual "new" will
> never return null pointers but instead will throw std::bad_alloc.
>
> Since the code in question is a COM method it is not allowed to let
> exceptions propagate through the COM boundary. So in current
> implementation if "new" ever runs out of memory an exception is
> thrown that is propagated to the consumer and causes undefined
> behaviour there - most likely the consumer will crash.
>
> That's no so bad by itself, but the very same behaviour would be
> there even without any checks. So current implementation only looks
> reliable - it checks for null to return E_OUTOFMEMORY which will just
> never happen, so that checking code is currently useless and only
> inflates the codebase.
>
> There're two possible solutions. Either "std::nothrow new" can be
> used - it will return null in case of no memory available. Or some
> construct for catching all exceptions can be used - something like
> _ATLTRY _ATLCATCHALL macros used everywhere in ATL in such cases. IMO
> the first variant (nothrow new) is better - cleaner, more
> straitforward and less changes. What's your opinion?

Committed your patch in r18336.
And you're right of course. I've added (std::nothrow) to a few new calls
in r18337.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2437522
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2010-01-15 19:30:23 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.