вторник, 28 сентября 2021 г.

[prog.philosophy] Показанный в предыдущем посте код пришлось усложнить еще больше

Вынужденное продолжение темы, поднятой в предыдущем посте. Код, который там был показан, пришлось усложнить еще больше. Сегодня попробую рассказать почему это произошло и что именно получилось на данный момент.

Напомню, что рассматривалась задача создания удобной C++ной обертки для манипуляции флагами Unix-овой функции fcntl. При этом хотелось сделать такой способ описания нужных для fcntl флагов, при котором можно указывать разные их комбинации (например, O_NONBLOCK и FD_CLOEXEC) и все это затем корректно трансформировалось бы в соответствующие вызовы fcntl.

Однако, первые несколько вариантов этой обертки использовались только для манипуляции флагами O_NONBLOCK и O_DIRECT, флаг FD_CLOEXEC на тот момент еще ни разу не применялся (его использование еще только-только рассматривалось в перспективе). И в этом-то и оказалась засада.

Вскоре после того, как "финальная" версия обертки была зафиксирована и использована, мне потребовалось немного модифицировать кусок старого кода, который был беззастенчиво скопирован в новый проект из "старых запасов". И вот тут-то я и наступил на грабли. Из фрагмента:

auto flags = ::fcntl(pipe_[0], F_GETFD, 0);
::fcntl(pipe_[0], F_SETFD, flags | FD_CLOEXEC);

был сделан фрагмент:

auto flags = ::fcntl(pipe_[0], F_GETFD, 0);
::fcntl(pipe_[0], F_SETFD, flags | FD_CLOEXEC | O_NONBLOCK);

после чего минут 20 ушло на разбирательство с тем, почему же ничего не работает.

Фокус в том, что флаг FD_CLOEXEC может использоваться только с командами F_GETFD/F_SETFD, тогда как O_NONBLOCK -- только с командами F_GETFL/F_SETFL. А комбинировать FD_CLOEXEC и O_NONBLOCK в одном вызове fcntl нельзя.

С одной стороны, это стало лишним напоминанием того, что когда API не использует типизацию для защиты от дурацких ошибок, то это гарантированно ведет к появлению этих самых дурацких ошибок. И тут вопрос стоит не в том, произойдет или нет. Произойдет. Вопрос лишь в том, когда и насколько быстро/просто/дешево это затем будет обнаружить и исправить.

Но, с другой стороны, это означало, что придуманное решение, к сожалению, не является финальным. Его нужно дорабатывать так, чтобы пользователь мог указать и O_NONBLOCK, и FD_CLOEXEC, и заданные пользователям значения затем успользовались бы в нескольких вызовах fcntl. Далее речь как раз и пойдет о том, что и как было сделано в итоге.

Как выглядит использование результирующего решения?

Вот так:

return make_anonymous_pipe(
      read_end_fcntl_flags()
         .set( fd_cloexec )
         .set( o_nonblock )
         .set( o_direct ),
      write_end_fcntl_flags()
         .clear( o_nonblock )
         .set( fd_cloexec )
         .set( o_direct ) );

Можно увидеть, что теперь каждый бит, который требуется установить/сбросить, задается отдельно, т.е. вместо set_bits(O_NONBLOCK|O_DIRECT) теперь нужно делать несколько вызовов set. Также, вместо стандартный Unix-овых имен, вроде O_NONBLOCK и FD_CLOEXEC, теперь применяются собственные имена o_nonblock и fd_cloexec. И это все не спроста :)

Ну а в общих чертах все осталось так же, как и было. О деталях же мы сейчас поговорим подробнее.

Некоторые детали новой реализации

Что произошло с шаблонным классом fcntl_flags_holder_t?

Поскольку выяснилось, что fcntl нужно вызывать с разными командами, то потребовалось раздельно хранить флаги, относящиеся к разным командам.

Делать это можно было по разному. Но на данный момент был выбран простой и дешевый путь: небольшой вектор значений для флагов, которые нужно сбросить, и такой же небольшой вектор значений для флагов, которые нужно установить:

template< anonymous_pipe_end >
class [[nodiscard]] fcntl_flags_holder_t
{
   static constexpr std::size_t max_dimension = 2u;

   //! Bits to be cleared for handle.
   std::array< int, max_dimension > m_bits_to_clear{ 00 };
   //! Bits to be set for handle.
   std::array< int, max_dimension > m_bits_to_set{ 00 };

Суть в том, что в m_bits_to_clear по нулевому индексу хранятся флаги для F_SETFD, а по индексу 1 -- флаги для F_SETFL. Аналогично и в m_bits_to_set.

На данный момент поддерживаются только F_GET/SETFD и F_GET/SETFL, поэтому в m_bits_to_clear/m_bits_to_set всего по два элемента. Если потребуется хранить еще что-то, то с этим придется разбираться отдельно и, возможно, содержимое класса fcntl_flags_holder_t будет расширено.

Соответственно, методы set/clear в fcntl_flags_holder_t просто добавляют бит для соответствующего элемента соответствующего массива:

template< fcntl_parameter_t Parameter >
auto &
clear( fcntl_parameter_value_t<Parameter> value ) & noexcept
{
   static_assert( to_index(Parameter) < max_dimension );

   m_bits_to_clear[ to_index(Parameter) ] |= value.get();
   return *this;
}

template< fcntl_parameter_t Parameter >
auto &
set( fcntl_parameter_value_t<Parameter> value ) & noexcept
{
   static_assert( to_index(Parameter) < max_dimension );

   m_bits_to_set[ to_index(Parameter) ] |= value.get();
   return *this;
}

Т.к. нам нужно брать значения для разных команд (т.е. для F_SETFD один набор флагов, для F_SETFL другой набор), то очевидный способ это реализовать getter-ы, которые будут возвращать флаги для конкретной команды:

[[nodiscard]]
auto
bits_to_clear( fcntl_parameter_t parameter ) const
{
   return m_bits_to_clear.at( to_index(parameter) );
}

[[nodiscard]]
auto
bits_to_set( fcntl_parameter_t parameter ) const
{
   return m_bits_to_set.at( to_index(parameter) );
}

Что используется как-то так:

flag_changer( result.read_end(),
      fcntl_parameter_t::file_descriptor,
      read_end_flags );
flag_changer( result.read_end(),
      fcntl_parameter_t::file_status,
      read_end_flags );

flag_changer( result.write_end(),
      fcntl_parameter_t::file_descriptor,
      read_end_flags );
flag_changer( result.write_end(),
      fcntl_parameter_t::file_status,
      read_end_flags );

 

Что такое fcntl_parameter_t и fcntl_parameter_holder_t?

В приведенных выше фрагментах можно увидеть новые типы fcntl_parameter_t и fcntl_parameter_holder_t, причем fcntl_parameter_holder_t -- это шаблон, который параметризуется значением fcntl_parameter_t. Выглядят эти типы следующим образом:

enum class fcntl_parameter_t
{
   //! F_GETFD/SETFD will be used.
   file_descriptor = 0,
   //! F_GETFL/SETFL will be used.
   file_status
};

template< fcntl_parameter_t >
class fcntl_parameter_value_t
{
   int m_value;

public:
   explicit constexpr
   fcntl_parameter_value_t( int v ) noexcept : m_value{ v } {}

   [[nodiscard]]
   constexpr int
   get() const noexcept { return m_value; }
};

Тип fcntl_parameter_t является всего лишь перечислением и он определяет какой именно набор команд fcntl должен использоваться для выставления/сброса битов.

А fcntl_parameter_value_t предназначен для хранения конкретных битов, которые нужно установить или сбросить. При этом, за счет параметризации элементом перечисления fcntl_parameter_t, значение увязывается с конкретной fcntl-шной командой.

Это дает нам возможность определить такие флаги, как O_NONBLOCK, O_DIRECT и FD_CLOEXEC следующим образом:

inline constexpr const
fcntl_parameter_value_t< fcntl_parameter_t::file_status > o_nonblock{ O_NONBLOCK };

inline constexpr const
fcntl_parameter_value_t< fcntl_parameter_t::file_status > o_direct{ O_DIRECT };

inline constexpr const
fcntl_parameter_value_t< fcntl_parameter_t::file_descriptor > fd_cloexec{ FD_CLOEXEC };

Благодаря таким определениям мы теперь точно знаем, что когда пользователь вызывает set(o_nonblock), то это означает установку бита O_NONBLOCK посредством команды F_SETFL.

При этом можно заметить, что в выражении set(o_nonblock) индекс в массиве fcntl_flags_holder_t::m_bits_to_set определяется параметром шаблона fcntl_parameter_value_t, т.е. в compile-time.

Нехорошая особенность C++, которую можно упомянуть в этом рассказе

Отличным добавлением в C++11 стало введение в язык enum class.

Однако, у enum class есть неоднозначная особенность: посредством static_cast можно приводить значения от перечисления к интегральному типу и обратно. Например, это очень полезно в связи с отсутствием в C++ strong typedefs:

enum class file_status_bits_t : int {};

void set_file_status_bits(file_status_bits_t bits) {
  const int actual_bits = static_cast<int>(bits);
  ...
}
...
set_file_status_bits(file_status_bits_t{O_NONBLOCK | O_DIRECT});

Здесь мы как бы выделили подтип int-а под именем file_status_bits_t и компилятор не позволит просто так передать int туда, где ожидается file_status_bits_t.

Однако, сама возможность сделать вот так:

enum class fcntl_parameter_t
{
   file_descriptor = 0,
   file_status
};

constexpr fcntl_parameter_t my_param{ 1024 };

без каких-либо предупреждений со стороны компилятора заставляет задуматься о том, а насколько надежным будет код вида:

template< fcntl_parameter_t Parameter >
auto &
clear( fcntl_parameter_value_t<Parameter> value ) & noexcept
{
   m_bits_to_clear[ to_index(Parameter) ] |= value.get();
   return *this;
}

С одной стороны, мы вроде как требуем, чтобы fcntl_parameter_value_t был параметризован валидным значением из перечисления fcntl_parameter_t. Поэтому, вроде как, можно и не парится на счет того, а валидным ли получится индекс из Parameter.

Но, с другой стороны, если кто-то по недосмотру сделал что-то вроде:

constexpr fcntl_parameter_t my_param{ 1024 };
...
read_end_fcntl_flags().set(fcntl_parameter_value_t<my_param>{0x81});

то компилятор никакого предупреждения не выдаст. И мы можем получить приключения в run-time, от которых хотели защититься той самой типизацией, рассказ о которой идет уже второй пост.

Поэтому-то и приходится вставлять в код ручные проверки, например:

template< fcntl_parameter_t Parameter >
auto &
clear( fcntl_parameter_value_t<Parameter> value ) & noexcept
{
   static_assert( to_index(Parameter) < max_dimension );

   m_bits_to_clear[ to_index(Parameter) ] |= value.get();
   return *this;
}

[[nodiscard]]
inline constexpr auto
get_fcntl_cmds( fcntl_parameter_t parameter )
{
   struct result_t { int m_query; int m_set; };

   switch( parameter )
   {
   case fcntl_parameter_t::file_descriptor:
      return result_t{ F_GETFD, F_SETFD };
   case fcntl_parameter_t::file_status:
      return result_t{ F_GETFL, F_SETFL };
   default:
      throw std::runtime_error{ "invalid fcntl_parameter value" };
   }
}

Так что лично я бы все-таки предпочел, чтобы в C++ был отдельно strong typedefs для случаев, когда нам нужны подтипы int-ов или unsigned long-ов. И отдельно enum class, который нельзя было бы перегонять в интегральные типы в compile-time ни через static_cast, ни еще каким-то легальным средством языка.

Так стоило ли оно того?

Итак, было проделано четыре удачных итераций вокруг fcntl_flags_holder_t. Удачных в том смысле, что получились работающие решения, которые решали поставленную задачу. И еще две попытки были прерваны на полпути, т.к. становилось понятно, что ничего хорошего из этого не выйдет.

Суммарно на удачные итерации было потрачено от 1.5 до 2 часов "чистого" времени. Из которого больше половины -- это документирование кода, связанного с fcntl_flags_holder_t.

Соответственно, ни одной ошибки в новом коде, где make_anonymous_pipe и fcntl_flags_holder_t применялись, допущено не было (или же пока не было выявлено).

С учетом того, что написанный вспомогательный код, скорее всего, сопровождать придется мне самому, то вряд ли реализованные "навороты" отрицательно скажутся на сложности/стоимости сопровождения. Скорее наоборот.

При этом можно учесть опыт поиска ошибки с комбинированием FD_CLOEXEC и O_NONBLOCK: разбирательство заняло вроде бы и немного времени, всего минут 20. Но за это время успел почувствовать себя полным идиотом и точно выбился из рабочего ритма по реализации новой функциональности.

Так что, наверное, на данном этапе все описанные эксперименты с написанием "надежной" make_anonymous_pipe, можно считать слишком дорогими. Вероятно, оно того не стоило.

Однако, есть две вещи, которые, на мой взгляд, все-таки компенсируют несколько чрезмерные трудозатраты:

  • во-первых, значительно более высокий коэффициент спокойного сна. Теперь лично у меня есть уверенность, что при использовании результирующей версии make_anonymous_pipe возможны логические ошибки, но исключены "очепятки по недосмотру". Так что в будущем при создании нового пайпа можно будет забыть указать флаг o_nonblock или, напротив, задать o_nonblock там, где он не нужен. Но вот чего нельзя будет получить, так это комбинацию O_NONBLOCK с F_SETFD. А именно такие проблемы искать будет сложнее всего;
  • во-вторых, был получен опыт, который в дальнейшем может сделать написание (или переиспользование) подобного кода совсем дешевым. Чтобы не быть голословным приведу совсем свежий пример. Несколько лет назад я в блоге показывал, как можно отрефакторить простой, но объемный код (тыц). И там был продемонстрирован класс handle_holder, который может хранить в RAII стиле разные типы хендлов. Так вот, в этом новом проекте данный класс был переиспользован, фактически, 1-в-1. Т.е. я тупо скопировал его и немного модифицировал имя класса и параметра шаблона, дабы соответствовать принятому сейчас стилю. Получается, что некий кусок работы был сэкономлен за счет того, что было сделано почти шесть лет назад.

Вместо заключения хочу поинтересоваться у тех читателей, которым хватило терпения добраться до этого места: а имеет ли смысл в блоге и впредь писать что-то подобное? Нашли ли вы для себя что-то интересное и полезное?

Если эта тема окажется невостребованной, то я бы больше не стал тратить на подобные описания ни свое, ни ваше время.

7 комментариев:

night beast комментирует...

Вместо заключения хочу поинтересоваться у тех читателей, которым хватило терпения добраться до этого места: а имеет ли смысл в блоге и впредь писать что-то подобное? Нашли ли вы для себя что-то интересное и полезное?

Много кода/текста.
Бегло пролистал не вникая в суть.
Имхо, столько писать нет смысла.

eao197 комментирует...

@night beast

Ok, спасибо.

С другой стороны, для чего-то короткого есть FB или Twitter. А блог как раз для того, чтобы "растечься мыслею по древу".

deadem комментирует...

> а имеет ли смысл в блоге и впредь писать что-то подобное?
Конечно, да! Конкретно тут не было откровений, но такие размышления иногда толкают мысль в правильном направлении. Поэтому - однозначно полезное!

eao197 комментирует...

@deadem:

Ok, спасибо!

MaxF комментирует...

Писать стоит. Тем более формат блога - это лонгрид в любом случае.

eao197 комментирует...

@MaxF:

Спасибо!

coredub комментирует...

У вас очень интересные посты :) Очень мне как то понравился "class file_mapped_memory" в стиле наворотов а-ля Андрей Александреску :)

Так что писать стоит :)