Re: [boost] [system] Was: Re: [asio] Breaking changes committed to cvs HEAD

From: Johan Nilsson (r.johan.nilsson_at_[hidden])
Date: 2006-11-14 04:41:04

Beman Dawes wrote:
> Johan Nilsson wrote:
>> Beman Dawes wrote:
>>> On 11/8/06, Johan Nilsson <r.johan.nilsson_at_[hidden]> wrote:
>>
>> [snip]
>>
>>>> I'm trying to find the documentation for the Boost.System stuff -
>>>> any pointers?
>>>>
>>> In the CVS Head, see libs/system/doc/error_code.html, and
>>> libs/system/doc/system_error.html
>>>
>>> Also see
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2066.html
>>
>> Ok, so I've made a quick pass though the docs. A few questions and
>> comments:
>>

[snip]

>
>> 2) I've implemented similar code previously, but only designed for
>> portability with linux and Win32. What I very often use, is something
>>  corresponding to retrieving errno or calling GetLastError (on
>> Win32).
>
> The C++ committee's Library Working Group is discussing making
> low-level functionality like that more generally available. It would
> be easy
> to implement.
>
>> With the current design, it is harder to implement library code
>> throwing system_errors (retrieving error codes) as you'll need to get
>> the corresponding error code in a platform-specific way. Or did I
>> miss something?
>>
>> I'd like something like this in the error_code interface:
>>
>> class error_code { ... static error_code
>> last_error_code(error_category const& ecat); static void
>> last_error_code(error_category const& ecat, error_code::value_type
>> new_error_code); ... };
>>
>> Which would enable:
>>
>> throw system_error(error_code::last_error_code(native_ecat),
>> "Failed!");
>>
>> of perhaps even:
>>
>> throw last_system_error("Failed!");
>>
>> I know it's not 100% natively portable, as e.g. OpenVMS does not have
>> an equivalent of Set/GetLastError - but at least using errno_ecat
>> should be universal, and using native_ecat could be implementation
>> defined (or simply use errno internally when nothing else is
>> available).
>
> Not quite sure what you mean here. All functions calling operating
> system API's either throw exceptions containing an error_code, or have
> non-throwing versions that return the error_code via an argument. So
> those cases are covered.
>
> If you are implementing your own library, presumably you are calling
> operating system dependent API's anyhow, so calling the operating
> system's "get last error" function shouldn't be a problem.

Imagine that you're implementing a "middle-layer" library, calling down to 
"lower-layer" methods that do the actual O/S specific calls. The lower-layer 
methods might be C methods, or a third-party library.

Examples for comparison:

---------(1)---------
void foo()
{
    if (!foo_impl())
    {
#ifdef BOOST_WINDOWS
        throw system_error(::GetLastError(), native_ecat, "Foo failed!");
#elif defined (...)
        throw system_error(errno, native_ecat, "Foo failed!");
#endif
    }
}
---------(2)---------
void foo()
{
    if (!foo_impl())
    {
        throw system_error(error_code::last_error_code(native_ecat), "Foo 
failed!");
    }
}
------------------

or perhaps:

---------(3)---------
void foo()
{
    if (!foo_impl())
    {
#ifdef BOOST_WINDOWS
        if (ERROR_SUCCESS != ::GetLastError())
        {
            ...
        }
#elif defined (...)
        if (0 != errno)
        {
            ...
        }
#endif
        ...
    }
}
---------(4)---------
void foo()
{
    if (!foo_impl())
    {
        if (error_code::last_error_code(native_ecat))
        {
            ...
        }
        ...
    }
}
------------------

[snip]

>
>>
>> 5) Ideally, I think having error_code as an additional part of
>> std::runtime_error instead of having the separate system_error class
>> would make sense.
>
> system_error is derived from std::runtime_error.

I know.

>
>> A error_code feels more like a property of a
>> runtime_error that might or might not be set. Might not be practical
>> though.
>
> If the error_code were part of the state of runtime_error rather than
> system_error, all uses of runtime_error would have to pay the
> (admittedly small) cost of having an error_code in each runtime_error
> object, even though only used for errors that have an associated
> error code.

Yes. I was simply stating a domain model ideal as opposed to a more 
"pragmatic" ideal (IMHO, of course).

>
>> 6) A "std::string error_category::description() const" method feels
>> like a good idea to add. Don't ask me for any specific reasons just
>> yet.
>
> How is that better than what()?

Well, there is no error_category::what() method.

[snip]

>
>> 8) An enumeration (or class with static constants) containing the
>> available standard posix error codes (as referenced in the docs)
>> would be nice, e.g. posix_error::xxx_errno, posix_error::yyy_errno,
>> ...
>>
>> 9) I assume the category management is thread-safe. I saw nothing
>> explicit about that in the docs.
>>
>> 10) I'd also like an explicit guarantee that the implementations of
>> error_category/system_error/error_code should be error neutral -
>> i.e., that they don't affect the current values of e.g errno and
>> GetLastError.
>>
>> 11) With reference to 10 above - a useful addition to the library
>> would be an "error_code_preserver" class, a RAII class that picks up
>> the current error code when created, and restores it on destruction.
>> Very useful when you want to implement your own error-neutral
>> functionality (such as in e.g. a logging framework). Perhaps nothing
>> for the standard, but why not for Boost.System?
>
> I'll also give those three some thought:-)

Four?

>
>> BTW, if you have implemented error_code::to_errno for native_ecat on
>> Win32, consider me suitably impressed.
>
> Nothing to be impressed about. It is just a "best guess" lookup table
> for the small number of errno values actually encountered by common
> API calls. Everything else just gets tossed into a catch-all code.

I imagined you had implemented a giant lookup-table, mapping native Win32 
codes to the closest matching POSIX errno equivalent.

>
>> Overall, this is for a very much awaited addition to Boost (and even
>> more so to the C++ standard, eventually). Looking forward to use it!
>
> Thanks!

You're welcome.

/ Johan