3

这是一个主观问题，所以我会接受“没有答案”，但请仔细阅读，因为这专门针对代码安全至关重要的系统。

我为安全关键系统采用了一些嵌入式 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 来别名和隐藏深层结构成员。大多是两三个，但偶尔也有四个深。顺便说一句：这是一个简单的例子，实际代码要复杂得多，但我不能在这里发布任何部分。

this 的使用并不一致，在某些地方别名变量直接使用其他变量的别名，某些代码部分没有别名。

IMO 这是一种不好的做法，因为它使代码变得模糊而无益，降低了可维护性和可读性，从而导致未来的错误和误解。

如果风格是 100% 一致的，那么也许我会更满意。

然而，作为安全关键的改变是昂贵的。因此，不想解决“不会破产”的问题，我对其他论点持开放态度。

我应该修复它还是不理会它？

是否有任何指南（例如通用 C、MISRA 或 DO178B 样式指南）对此有意见？

4

3 回答 3

2

然而，作为安全关键的改变是昂贵的。因此，不想解决“不会破产”的问题，我对其他论点持开放态度。

最关键的代码得到的关注最少，这是自相矛盾的死亡螺旋，因为人们害怕改变它。

你犹豫要不要对这段代码进行简单的、死记硬背的重构，这告诉我代码要么没有测试，要么你不信任这些测试。当你害怕改进代码因为你可能会破坏它时，就会延迟对代码的改进。您可能会做尽可能小的事情，这会使代码更加脆弱和不安全。

我建议第一件事是进行一些测试以及用于试验的暂存环境。然后所有更改都变得更安全。当你发现这段代码正在做的所有奇怪和危险的事情时，最初可能会有一些失态，但这就是暂存区的用途。从中长期来看，每个人都会更快、更有信心地改进此代码。使代码更容易和更安全地更改可以使其更容易和更安全地更改；然后螺旋上升，而不是下降。

使宏看起来像单个变量的技术是我以前在 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)因为它从未出现在您的源代码中。

与许多宏 hack 一样，虽然该技术非常聪明，但对许多程序员来说也是令人费解且不熟悉的。在安全关键代码中，思维弯曲不是您想要的。你想要简单、无聊的代码。仅此一项就是用命名良好的 getter 和 setter 函数替换它的最简单的参数。相信编译器会优化它们。

这方面的一个很好的例子是 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为指针以节省内存）。您正在使用的变量很清楚，使整个代码更容易理解。函数名称清楚地解释了正在发生的事情。因为它们是函数，所以它们有一个明显的文档位置。相信编译器知道如何最好地优化它。

于 2019-04-28T19:49:38.777 回答
1

从维护的角度来看，是的，这绝对是需要修复的代码。

但是，只有从这个角度来看，代码才需要修复。它不会损害程序的正确性，如果代码按原样正确，那是最重要的考虑因素。

这就是为什么像这样的代码永远不应该被修复，除非一个彻底的单元测试和回归测试方案已经到位。如果您可以确定在此过程中没有破坏正常运行的代码，您应该只修复这样的代码。

于 2019-04-28T19:14:47.787 回答
1

是的，你应该摆脱它。晦涩的宏是危险的。

在较旧的 C 语言中，避免拼写深度嵌套来执行以下操作是很常见的

#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是一个范围有限的局部变量。这比晦涩的宏更具可读性，并且在机器代码中得到了优化。

于 2019-04-29T07:05:45.467 回答