Commit f46dafa3 authored by Ramon Nou's avatar Ramon Nou
Browse files

Merge branch 'amiranda/81-make-admire-error_code-a-fully-fledged-class' into 'main' 

Resolve "Make `admire::error_code` a fully fledged class"

This MR redefines the `admire::error_code` typedef transforming it into
a fully-fledged C++ object.

This has the following advantages:

1. The default `error_code` constructor initializes to `ADM_SUCCESS`.
   Thus, we can now do `error_code ec;` instead of `error_code ec = ADM_SUCCESS;`.
2. `error_code`s can be constructed explicitly from `ADM_return_t` values.
3. `error_code`s can be constructed explicitly from `int32_t` (for RPC types).
4. `error_code`s can be converted implicitly to `ADM_return_t`, which makes code
   less verbose when returning from C++ code to the C code.
5. `error_code`s are convertible to `bool`. This means that we can now do

   ```c++
       error_code ec = fun();
       if(ec) { /* there was an error, do something */ }
   ```
   
   whereas previously we were forced to explicitly check against `ADM_SUCCESS`.

   ```c++
       error_code ec = fun();
       if(ec != ADM_SUCCESS) { /* there was an error, do something */ }
   ```

6. `error_code`s provide `value()`, `name()`, and `message()` functions that make
   them much more flexible.
7. Several `error_code` constants are defined to avoid using `ADM_return_t` 
   directly in C++ code, hopefully making the code more readable:
   
   ```c++
      constexpr error_code error_code::success = error_code{ADM_SUCCESS};
      constexpr error_code error_code::snafu = error_code{ADM_ESNAFU};
      constexpr error_code error_code::bad_args = error_code{ADM_EBADARGS};
      constexpr error_code error_code::out_of_memory = error_code{ADM_ENOMEM};
      constexpr error_code error_code::entity_exists = error_code{ADM_EEXISTS};
      constexpr error_code error_code::no_such_entity = error_code{ADM_ENOENT};
      constexpr error_code error_code::adhoc_in_use = error_code{ADM_EADHOC_BUSY};
      constexpr error_code error_code::other = error_code{ADM_EOTHER};
   ```

Also, most of the implementation is `constexpr`, which is always nice.

Closes #81

See merge request !61
parents 0658d9bd 0480c839

src/common/api/CMakeLists.txt

+1 −1
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ 
add_library(_api_types STATIC)



target_sources(_api_types PUBLIC admire_types.h admire_types.hpp PRIVATE

  types.cpp convert.hpp convert.cpp internal_types.hpp)

  types.cpp convert.hpp convert.cpp internal_types.hpp errors.c)



target_include_directories(_api_types PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})

src/common/api/admire_types.hpp

+69 −33
Original line number Diff line number Diff line
@@ -36,7 +36,74 @@ 


namespace admire {



using error_code = ADM_return_t;

struct error_code {



    static const error_code success;

    static const error_code snafu;

    static const error_code bad_args;

    static const error_code out_of_memory;

    static const error_code entity_exists;

    static const error_code no_such_entity;

    static const error_code adhoc_in_use;

    static const error_code other;



    constexpr error_code() : m_value(ADM_SUCCESS) {}

    constexpr explicit error_code(ADM_return_t ec) : m_value(ec) {}

    constexpr explicit error_code(int32_t ec)

        : m_value(static_cast<ADM_return_t>(ec)) {}



    constexpr operator ADM_return_t() const { // NOLINT

        return m_value;

    }



    constexpr explicit operator bool() const {

        return m_value == ADM_SUCCESS;

    }



    ADM_return_t

    value() const {

        return m_value;

    }



    constexpr std::string_view

    name() const {

        switch(m_value) {

            case ADM_SUCCESS:

                return "ADM_SUCCESS";

            case ADM_ESNAFU:

                return "ADM_ESNAFU";

            case ADM_EBADARGS:

                return "ADM_EBADARGS";

            case ADM_ENOMEM:

                return "ADM_ENOMEM";

            case ADM_EEXISTS:

                return "ADM_EEXISTS";

            case ADM_ENOENT:

                return "ADM_ENOENT";

            case ADM_EADHOC_BUSY:

                return "ADM_EADHOC_BUSY";

            case ADM_EOTHER:

                return "ADM_EOTHER";

            default:

                return "INVALID_ERROR_VALUE";

        }

    }



    std::string_view

    message() const;



private:

    ADM_return_t m_value;

};



constexpr error_code error_code::success = error_code{ADM_SUCCESS};

constexpr error_code error_code::snafu = error_code{ADM_ESNAFU};

constexpr error_code error_code::bad_args = error_code{ADM_EBADARGS};

constexpr error_code error_code::out_of_memory = error_code{ADM_ENOMEM};

constexpr error_code error_code::entity_exists = error_code{ADM_EEXISTS};

constexpr error_code error_code::no_such_entity = error_code{ADM_ENOENT};

constexpr error_code error_code::adhoc_in_use = error_code{ADM_EADHOC_BUSY};

constexpr error_code error_code::other = error_code{ADM_EOTHER};



using job_id = std::uint64_t;

using slurm_job_id = std::uint64_t;
@@ -439,38 +506,7 @@ struct fmt::formatter<admire::error_code> : formatter<std::string_view> { 
    template <typename FormatContext>

    auto

    format(const admire::error_code& ec, FormatContext& ctx) const {

        std::string_view name = "unknown";



        switch(ec) {

            case ADM_SUCCESS:

                name = "ADM_SUCCESS";

                break;

            case ADM_ESNAFU:

                name = "ADM_ESNAFU";

                break;

            case ADM_EBADARGS:

                name = "ADM_EBADARGS";

                break;

            case ADM_ENOMEM:

                name = "ADM_ENOMEM";

                break;

            case ADM_EEXISTS:

                name = "ADM_EEXISTS";

                break;

            case ADM_ENOENT:

                name = "ADM_ENOENT";

                break;

            case ADM_EADHOC_BUSY:

                name = "ADM_EADHOC_BUSY";

                break;

            case ADM_EOTHER:

                name = "ADM_EOTHER";

                break;

            default:

                break;

        }



        return formatter<std::string_view>::format(name, ctx);

        return formatter<std::string_view>::format(ec.name(), ctx);

    }

};

src/lib/errors.c→src/common/api/errors.c

+1 −1
Original line number Diff line number Diff line
@@ -22,7 +22,7 @@ 
 * SPDX-License-Identifier: GPL-3.0-or-later

 *****************************************************************************/



#include <admire.h>

#include <admire_types.h>



const char* const adm_errlist[ADM_ERR_MAX + 1] = {

        [ADM_SUCCESS] = "Success",

src/common/api/internal_types.hpp

+2 −2
Original line number Diff line number Diff line
@@ -86,12 +86,12 @@ struct adhoc_storage_info { 
        if(m_client_info) {

            LOGGER_ERROR("adhoc storage {} already has a client",

                         m_adhoc_storage.id());

            return ADM_EADHOC_BUSY;

            return error_code::adhoc_in_use;

        }



        m_client_info = std::move(job_info);



        return ADM_SUCCESS;

        return error_code::success;

    }



    void

src/common/api/types.cpp

+10 −0
Original line number Diff line number Diff line
@@ -1013,8 +1013,18 @@ ADM_qos_limit_list_destroy(ADM_qos_limit_list_t list) { 
/* C++ Type definitions and related functions                                 */

/******************************************************************************/



extern "C" {

const char*

ADM_strerror(ADM_return_t errnum);

};



namespace admire {



std::string_view

error_code::message() const {

    return ::ADM_strerror(m_value);

}



class server::impl {



public: