понедельник, 5 июня 2023 г.

[proc.flame] Писать код нужно так, чтобы было очевидно, что в нем нет ошибок, или же так чтобы...

...было неочевидно, есть ли в нем ошибки.

Афоризм классный, жизненный, не мой :)

В том, что он жизненный, мне довелось убедиться на днях. Захотелось посмотреть, насколько безопасно передавать уже использовавшийся AVPacket в av_read_frame. Здравый смысл подсказывал, что должно быть безопасно и что внутри av_read_frame сам FFMPEG должен вызывать av_packet_unref(). Но хотелось убедиться.

Заглянул в потроха FFMEG, дошел до внутренней функции read_frame_internal и слегка офигел. Этот самый афоризм тут же вспомнился. В той его части, которая про "неочевидно, есть ли в нем ошибки" :(

Ну, например: если смотреть по диагонали, то кажется, что перед возвратом ошибки делается вызов av_packet_unref() для очистки содержимого AVPacket. Вот как здесь. Но если попробовать присмотреться, то находятся места, где это не так (тыц или тыц). И ведь сходу и не поймешь, правильно ли это или нет. Неочевидненько...

Был бы я помоложе, наверняка бы высказал свое "фи" в той или иной форме. Но годы таки берут свое и сейчас я уже вовсе не уверен в правильности своих оценок. Поэтому смотрю я на эти 200 строк одной функции и не понимаю: программирование в таком духе -- это нормально или нет? Может это просто я безнадежно застрял в своем манямирке и уже не представляю, как пишется промышленный код в больших проектах?

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


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

Это вообще больной вопрос, когда приходится сталкиваться с Си: вроде бы код на Си читаю и понимаю что происходит. Но стоит задуматься о том, как на чистом Си что-то реализовать, как возникает ощущение, что не понимаю, как на этом языке вообще можно что-то надежно и качественно писать. Тут ведь все на внимании и скрупулезности программиста зиждется. А вот чего у меня нет, того нет... :(

В рамках C++ первое, что пришло в голову, так это создание вспомогательной обертки, в которую бы передавался указатель на AVPacket и в которой бы содержался признак актуальности значения по этому указателю (назовем этот признак committed). В деструкторе этой обертке проверялось бы значение committed и, если оно не выставлено, то для AVPacket вызывался бы av_packet_unref:

class AVPacketCleaner {
  AVPacket * pkt_;
  bool committed_{false};
public:
  AVPacketCleaner(AVPacket * pkt) : pkt_{pkt} {}
  ~AVPacketCleaner() { if(!committed_) av_packet_unref(pkt_); }
  void commit() noexcept { committed_ = true; }
};

Тогда бы в read_frame_internal (или даже выше, еще в av_read_frame) можно было бы написать что-то вроде:

int av_read_frame(AVFormatContext * s, AVPacket * pkt) {
  ...
  AVPacketCleaner cleaner{pkt};
  ...
  // Перед тем return-ом, который возвращает признак успеха:
  cleaner.commit();
  return 0;
err:
  ... // Обработка ошибок.
}

И все, не пришлось бы париться на предмет того, следует ли при очередном возврате кода ошибки вызывать вручную av_packet_unref() или нет.

Вторая идея, которая пришла в голову чуть позже: это использование идиомы swap с временным значением. Т.е., чтобы было что-то вроде:

int av_read_frame(AVFormatContext * s, AVPacket * pkt) {
  ...
  AVPacket tmp_pkt{...}; // Создание временного экземпляра.
  ... // Далее работа только с tmp_pkt.
  // Перед тем return-ом, который возвращает признак успеха:
  swap(tmp_pkt, *pkt);
  return 0;
err:
  ... // Обработка ошибок.
}

При этом подразумевается, что у AVPacket есть деструктор, который вызывает av_packet_unref().

Т.е., за годы работы на C++ выработалась привычка использовать возможности C++ так, чтобы код требовал меньшего внимания с моей стороны и всякий мелкий (и не очень) мусор бы подчищался бы автоматически. Но в стандартном чистом Си ничего подобного нет, поэтому даже сама мысль о том, чтобы что-то запрограммировать на "теплой и ламповой Сишечке", пробивает на холодный пот ;)

1 комментарий: