Re: [boost] [system] Was: Re: [asio] Breaking changescommittedtocvs HEAD

From: Johan Nilsson (r.johan.nilsson_at_[hidden])
Date: 2006-11-15 09:48:56

Christopher Kohlhoff wrote:
> Hi Johan,
>
> Johan Nilsson <r.johan.nilsson_at_[hidden]> wrote:
>> Extract from that original posting:
>>
>> Guidelines:
> [...]
>>    Such functions should also have an overload that takes an
>>    additional argument of type boost::system_error_code& ec.
>>    The behavior of this overload is the same as the
>>    non-overloaded version, except that it does not throw an
>>    exception on an API error, and it sets ec to the error code
>>    reported by the operating system, or to 0 if no error is
>>    reported.
> [...]
>> The second paragraph leads to the implementation being exposed
>> through its interface.
>
> I'm a little confused by this statement. Can you expand on it?

I just meant that, according to the paragraph, by adding a error_code 
argument to the method you are saying that this method is implemented by 
delegating to system specific APIs.

> I
> don't understand how it's relevant to the discussion, i.e. how
> it's different from a function that sets errno (or equivalent).

I guess it is not very relevant for this discussion. The difference is 
perhaps the one between explicit (error_code) and implicit (errno). I'm just 
not very fond of providing function overloads for the sake of adding an 
error_code argument.

>
>> OTOH, usage of error_codes could also cause that kind of
>> latent errors:
>>
>> -----------
>> error_code const& foo(error_code& ec)
>> {
>>     if (!SomeWindowsAPICall())
>>     {
>>         return ec; // oops, forgot to set ec
>>     }
>>
>>     ... do something ...
>>     ec = error_code(); // success
>>
>>     return ec;
>> }
>> ----------
>
> Hmm, I wouldn't class that as the same kind of error at all as
> the problem is clearly evident in the function itself -- it
> doesn't involve hidden side effects of a separate function. The
> error code value represented by errno/GetLastError is a
> thread-specific global, and IMHO is bad for the same reasons as
> why globals are considered bad.

For me personally, the problem is equally evident in the first scenario. I 
guess that for many users it is not, though.

>
> On a related note, this whole issue with functions having the
> side effect of modifying errno is the rationale for an
> error_code_preserver class. If you don't have the side effects
> in the first place, you don't need an error_code_preserver
> either.

Agreed, but the side effects are unescapable when using API functions.

>
>> I'm not "blessing" anything.
>>
>> Also, the TSS-based suggestion had nothing to do with my
>> "don't pay for what you don't use" statement - that was only
>> referring to the Set/GetLastError/errno wrapper. Those might
>> be TSS-based (or equivalently implemented), but that was not
>> my point.
>
> I meant that putting a portable wrapper for errno / GetLastError
> / SetLastError in the public interface of the system library
> would effectively bless that style of error handling for use by
> a wider C++ audience. I'm not in favour of that.

I'm not blessing a TSS-based style of _error handling_, I originally just 
expressed my desire  (perhaps badly) to have access to that functionality in 
the role as a library implementor.

I'd be happy enough if such wrappers would be documented and existed in 
<boost/system/convenience.hpp> or similar.

>
> BTW, I was just using "TSS-based" as a shorthand for errno and
> GetLastError (or any wrapper of them) that behaves as though
> there is a separate error code per thread, and not a comment on
> how they're implemented. Perhaps "thread-specific error code" is
> a clearer term.

Both terms are clear enough to me.

Regards / Johan