Если макрос находится в исходном файле и используется только в этом файле, то я нахожу его немного менее оскорбительным, чем если бы он был выключен в каком-то заголовке. Но мне не очень нравится макрос, который возвращает (особенно тот, который задокументирован , чтобы завершить приложение, и фактически возвращает), и еще меньше, который возвращает условно, потому что он довольно легко создает следующая утечка памяти:
char *buf = malloc(1000);
if (buf == 0) return ENOMEM;
ret_val = read(fd, buf, 1000);
CHECK_READ(ret_val, "buffer read failed")
// do something with buf
free(buf);
Если я верю документации, у меня нет оснований подозревать, что этот код теряет память при сбое чтения. Даже если документация была правильной, я предпочитаю, чтобы поток управления в C был явно очевидным, то есть не скрытым макросами. Я также предпочел бы, чтобы мой код выглядел смутно согласованным между случаями, когда у меня есть ресурсы для очистки, и случаями, когда я этого не делаю, вместо того, чтобы использовать макрос для одного, но не другого. Так что макрос мне не по вкусу, даже если он не является оскорбительным.
Чтобы сделать то, что написано в документации, я бы предпочел написать что-то вроде:
check(retval != -1, "buffer read failed");
с check
функцией. Или используйте assert
. Конечно, assert делает что-либо, только если NDEBUG не установлен, поэтому для обработки ошибок не годится, если вы планируете отдельные сборки отладки и выпуска.
Чтобы сделать то, что на самом деле делает макрос, я бы предпочел полностью пропустить макрос и написать:
if (retval == -1) return DCD_BADREAD;
Это вряд ли длинная строка кода, и нет никакой реальной выгоды в обобщении различных его экземпляров. Также, вероятно, не стоит думать, что вы улучшите ситуацию, заключив в капсулу / скрывая существующие, хорошо документированные средства, с помощью которых read
указывает на ошибки, что широко понимают программисты на Си. Если ничего другого, эта проверка полностью игнорирует тот факт, что read
может выдать меньше данных, чем вы просили, без видимой причины. Это не ошибка макроса напрямую, но вся идея макроса, похоже, исходит из плохой проверки ошибок.
Вероятно, здесь произошло то, что у них был макрос, который завершился, затем они решили, что не хотят прерывать программу сразу же при возникновении сбоев, вместо этого они хотели вернуть этот код ошибки DCD_BADREAD. Они должны были полностью удалить макрос и изменить все вызывающие сайты - если вы измените логику управления функцией и измените то, что она возвращает, лучше всего заметно изменить источник этой функции. Вместо этого они сделали минимально возможное изменение, которое дало желаемый результат. Возможно, в то время это казалось хорошей идеей (и, к сожалению, забывать обновлять комментарии, к сожалению, является распространенной ошибкой). Но, как все, кажется, согласны, это привело к очень хитрому коду.
Макросы, которые управляют потоком могут быть не оскорбительными, но я думаю, что только в ситуациях, когда они выглядят как поток управления и точно задокументированы. Вспоминаются макрокомандные макросы Саймона Тэтхэма - некоторым людям изначально не нравятся сопрограммы, но, если вы примете предпосылку, макрос с именем crReturn
по крайней мере выглядит так, как будто вернуться.