Использование #define для псевдонимов членов структуры

Это субъективный вопрос, поэтому я приму "ответа нет", но прочитал полностью, так как это специально для системы, где код критичен по безопасности.

Я принял некоторый встроенный C-код для критической системы безопасности, где первоначальный автор (в случайных местах) использовал такой синтаксис:

#include <stdio.h>

typedef struct tag_s2 {
    int a;
}s2;

typedef struct tag_s1 {
  s2 a;
}s1;

s1 inst;

#define X_myvar inst.a.a

int main(int argc, char **argv)
{
   X_myvar = 10;
   printf("myvar = %d\n", X_myvar + 1);
   return 0;
}

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

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

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

Если бы стиль был на 100% последовательным, то, возможно, я был бы более счастлив с ним.

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

Должен ли я это исправить или оставить в покое?

Есть ли какое-либо руководство (например, руководства в стиле Generic C, MISRA или DO178B), в котором есть мнение по этому поводу?

3 ответа

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

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

#define a inst.a

В этом случае вам нужно было только набрать inst.a вместо inst.a.a, Хотя это сомнительная практика, подобные макросы использовались для исправления недостатка в языке, а именно отсутствия анонимных структур. Современный C поддерживает это из C11, хотя. Мы можем использовать анонимные структуры, чтобы избавиться от ненужных вложенных структур:

typedef struct {
  struct
  { 
    int a;
  };
}s1;

Но MISRA-C:2012 не поддерживает C11, так что это может быть не вариант.

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

int* x = &longstructname.somemember.anotherstruct.x; 
// use *x from here on

x является локальной переменной с ограниченной областью действия Это гораздо более читабельно, чем непонятные макросы, и оно оптимизируется в машинном коде.

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

Парадоксальная смертельная спираль в том, что наиболее критичный код получает наименьшее внимание, потому что люди боятся его менять.

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

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


Техника создания макроса, похожего на одну переменную, - это метод, который я видел ранее в кодовой базе Perl 5. Он написан больше на макросах C, чем на C. Например, вот немного манипулирования стеком вызовов Perl.

#define SP sp
#define MARK mark
#define TARG targ

#define PUSHMARK(p) \
    STMT_START {                                                      \
        I32 * mark_stack_entry;                                       \
        if (UNLIKELY((mark_stack_entry = ++PL_markstack_ptr)          \
                                           == PL_markstack_max))      \
        mark_stack_entry = markstack_grow();                      \
        *mark_stack_entry  = (I32)((p) - PL_stack_base);              \
        DEBUG_s(DEBUG_v(PerlIO_printf(Perl_debug_log,                 \
                "MARK push %p %" IVdf "\n",                           \
                PL_markstack_ptr, (IV)*mark_stack_entry)));           \
    } STMT_END

#define TOPMARK S_TOPMARK(aTHX)
#define POPMARK S_POPMARK(aTHX)

#define INCMARK \
    STMT_START {                                                      \
        DEBUG_s(DEBUG_v(PerlIO_printf(Perl_debug_log,                 \
                "MARK inc  %p %" IVdf "\n",                           \
                (PL_markstack_ptr+1), (IV)*(PL_markstack_ptr+1))));   \
        PL_markstack_ptr++;                                           \
} STMT_END
#define dSP     SV **sp = PL_stack_sp
#define djSP        dSP
#define dMARK       SV **mark = PL_stack_base + POPMARK
#define dORIGMARK   const I32 origmark = (I32)(mark - PL_stack_base)
#define ORIGMARK    (PL_stack_base + origmark)

#define SPAGAIN     sp = PL_stack_sp
#define MSPAGAIN    STMT_START { sp = PL_stack_sp; mark = ORIGMARK; } STMT_END

#define GETTARGETSTACKED targ = (PL_op->op_flags & OPf_STACKED ? POPs : PAD_SV(PL_op->op_targ))
#define dTARGETSTACKED SV * GETTARGETSTACKED

Это макросы на макросах на макросах. Источник Perl 5 изобилует ими. Там происходит много непрозрачной магии. Некоторые из них должны быть макросами, чтобы разрешить присваивание, но многие могут быть встроенными функциями. Несмотря на то, что они являются частью общедоступного API, они частично отражаются в документации, поскольку они являются макросами, а не функциями.

Этот стиль очень умный и полезный, если вы уже хорошо знакомы с исходным кодом Perl 5. Для всех остальных это сделало работу с внутренностями Perl 5 чрезвычайно сложной. В то время как некоторые компиляторы будут предоставлять трассировки стека для расширений макросов, другие будут сообщать только о расширенном макросе, оставляя одного поцарапанного в голове, какого черта const I32 origmark = (I32)(mark - PL_stack_base) потому что это никогда не появляется в вашем источнике.

Как и многие макрокоманды, хотя техника очень умная, она также ошеломляет и незнакома многим программистам. Сгибание ума - это не то, что вам нужно в критически важном коде. Вы хотите простой, скучный код. Одно это - самый простой аргумент, чтобы заменить его хорошо именованными функциями получения и установки. Доверьтесь компилятору, чтобы оптимизировать их.


Хорошим примером этого является GLib, которая тщательно использует хорошо документированные функциональные макросы для создания общих структур данных. Например, добавление значения в массив.

#define             g_array_append_val(a,v)

Хотя это макрос, он действует и документируется как функция. Это макрос исключительно как механизм для создания безопасного типа универсального массива. Он не скрывает никаких переменных. Вы можете безопасно использовать его, даже не зная, что это макрос.


В заключение да, измените это. Но вместо простой замены X_myvar с inst.a.a рассмотреть возможность создания функций, которые продолжают обеспечивать инкапсуляцию.

void s1_set_a( s1 *s, int val ) {
    s->a.a = val;
}

int s1_get_a( s1 *s ) {
    return s->a.a;
}

s1_set_a(&inst, 10);
printf("myvar = %d\n", s1_get_a(&inst) + 1);

Внутренние органы s1 скрыты, что облегчает изменение внутренних органов позже (например, изменение s1.a на указатель для экономии памяти). С какой переменной вы работаете, становится понятным весь код. Имена функций дают четкое объяснение того, что происходит. Поскольку они являются функциями, у них есть очевидное место для документации. Доверьтесь компилятору, чтобы узнать, как лучше его оптимизировать.

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

Однако только с этой точки зрения код должен быть исправлен. Это не вредит правильности программы, и, если код верен как есть, это первостепенное значение.

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

Другие вопросы по тегам