c-MISRA2012规则12.1建议使用额外的括号

我正试图修复我的代码对misra C的兼容性。在静态分析过程中，我遇到了以下违规：

规则12.1：建议使用额外的括号。条件运算是另一个条件运算符的操作数。

代码为：

if (CHANNEL_STATE_GET(hPer, channel) != CHANNEL_STATE_READY)
{
retCode = ERROR;
}

其中CHANNEL_STATE_GET是如下所示的宏：

#define CHANNEL_STATE_GET(__HANDLE__, __CHANNEL__)
(((__CHANNEL__) == CHANNEL_1) ? (__HANDLE__)->ChannelState[0] :
((__CHANNEL__) == CHANNEL_2) ? (__HANDLE__)->ChannelState[1] :
((__CHANNEL__) == CHANNEL_3) ? (__HANDLE__)->ChannelState[2] :
((__CHANNEL__) == CHANNEL_4) ? (__HANDLE__)->ChannelState[3] :
((__CHANNEL__) == CHANNEL_5) ? (__HANDLE__)->ChannelState[4] :
(__HANDLE__)->ChannelState[5])

你有解决这个违规问题的想法吗？

BR，Vincenzo

就MISRA C而言，这里有几个问题：

有各种规则说宏和复杂表达式应该用括号括起来，代码不应该依赖于C程序员知道每个运算符的优先规则。你可以通过在表达式上添加更多的括号来解决这个问题，但这只是冰山一角
CCD_ 1运算符被认为是"1"运算符；复合运算符"；因此包含它的表达式被认为是"；复合表达式"；并附带了一系列额外的规则10.6、10.7和10.8。这意味着，关于这个宏何时以及如何与其他表达式混合，有很多规则——主要关注的是隐式、意外的类型转换
首先应该避免使用类似函数的宏
C语言不允许使用以多个下划线开头的标识符，因为它为实现保留了这些标识符(C17 7.1.3)

更简单且推荐的解决方案是忘记该宏，因为它只会导致大规模的MISRA合规性头痛。此外，乍一看，它看起来像是具有嵌套分支的效率非常低的代码。我建议的解决方案：

如果hPer恰好是指向指针的指针(看起来像)，则取消引用它，并将结果存储在一个普通的临时指针变量中。不要在整个函数/宏中拖动讨厌的指针到指针语法
将整个宏替换为(内联)函数或纯数组表查找，具体取决于对channel索引的清理程度
确保CHANNEL_1到CHANNEL_5是从0到4的相邻整数。如果不是，请使用其他常量或中间的查找

符合MISRA的重新设计可能如下所示：

typedef enum
{
CHANNEL_1,
CHANNEL_2,
CHANNEL_3,
CHANNEL_4,
CHANNEL_5
} channel_t;
// state_t is assumed to be an enum too
state_t CHANNEL_STATE_GET (const HANDLE* handle, channel_t channel)
{
if((uint32_t)channel > (uint32_t)CHANNEL_5)
{
/* error handling here */
}
uint32_t index = (uint32_t)channel;
return handle[index];
}
...
if (CHANNEL_STATE_GET(*hPer, channel) != CHANNEL_STATE_READY)

如果您可以信任channel的值，那么您甚至不需要该函数，只需查找表即可。还应注意，MISRA C鼓励"；手柄；在这种情况下是不透明的类型，但这是它自己的一章。

请注意，这段代码还假设HANDLE不是像Windows API等中那样隐藏在typedef后面的指针——如果是这样，那么也需要修复。

注意(正如Lundins评论中或多或少暗示的那样……)，我回答了更多关于如何处理MISRA发现的问题(以及我遇到的其他一些分析工具的问题……)

我会首先尝试从一个更好的角度了解这一发现的实际描述。对于如图所示的嵌套结构，这需要重新审视。所以

我会应用缩进，只是为了在编辑时让生活更轻松，然后，在吸引人的地方再添加一些()，例如，在这种情况下，将每个x?y:z封装成一对。

#define CHANNEL_STATE_GET(__HANDLE__, __CHANNEL__)
(        ((__CHANNEL__) == CHANNEL_1) ? (__HANDLE__)->ChannelState[0] :
(      ((__CHANNEL__) == CHANNEL_2) ? (__HANDLE__)->ChannelState[1] :
(    ((__CHANNEL__) == CHANNEL_3) ? (__HANDLE__)->ChannelState[2] :
(  ((__CHANNEL__) == CHANNEL_4) ? (__HANDLE__)->ChannelState[3] :
(((__CHANNEL__) == CHANNEL_5) ? (__HANDLE__)->ChannelState[4] :
(__HANDLE__)->ChannelState[5]  
)                                                              
)                                                                
)                                                                  
)                                                                    
)

这是为了说明引用的发现是关于什么的
如果再多洒几滴，我也不会觉得不好，例如每个?:0。

(我承认，我没有针对MISRA检查器测试我的代码。我试图提供一种方法。我希望这能修复上面提到的发现，可能会用另一个替换它……根据我的经验，MISRA擅长这一点……我甚至不希望这能解决所有发现。)

当试图修复像这样非常奇怪的代码时，通常最好后退一两大步。

我们知道hPer指的是一个数组。我们有一些麻烦的代码正在索引到该数组中，并提取其中一个通道状态。但坦率地说，这个代码非常糟糕。即使MISRA检查器没有对此抱怨，但每当你有五个嵌套的?:运算符，对本应是简单的数组查找进行繁琐的手工模拟时，这肯定是一个迹象，表明有些地方不对劲，而且可能有更好的方法来做这件事。那么，更好的方法是什么呢？

解决这个问题的一种方法是问，ChannelState数组是如何填充的？还有其他代码可以从中提取吗

您只向我们询问了您的MISRA检查器正在抱怨的这一行。这表明，填充ChannelState数组的代码以及从中提取的任何其他代码都不会引起抱怨。也许其他代码以某种不同的方式访问ChannelState数组，希望是更好的方式。也许潜在的问题是，编写这个CHANNEL_STATE_GET宏的程序员不知道其他代码，没有接受过关于这个程序的编码约定和可用实用程序的适当教育。也许使用channel值直接为ChannelState数组编制索引是完全可以接受的。或者可能已经有了类似map_channel_index函数的东西，我在另一个答案中建议了它。

所以，帮自己一个忙：花几分钟时间寻找访问ChannelState数组的其他代码。你可能会学到一些非常有趣的东西。

其他评论和答案建议用简单得多的数组查找取代繁琐的CHANNEL_STATE_GET宏，我强烈同意这一建议。

不过，CHANNEL_1到CHANNEL_5的定义可能不在您的控制之下，因此您无法保证它们是所需的连续小整数。在这种情况下，我建议编写一个小函数，它的唯一任务是将channel_t映射到数组索引。最明显的方法是使用switch语句：

unsigned int map_channel_index(channel_t channel)
{
switch(channel) {
case CHANNEL_1: return 0;
case CHANNEL_2: return 1;
case CHANNEL_3: return 2;
case CHANNEL_4: return 3;
case CHANNEL_5: return 4;
default:        return 5;
}
}

然后你可以定义更简单的

#define CHANNEL_STATE_GET(handle, channel) 
((handle)->ChannelState[map_channel_index(channel)])

或者，您可以通过替换来完全摆脱CHANNEL_STATE_GET

if(CHANNEL_STATE_GET(hPer, channel) != CHANNEL_STATE_READY)

带有

if((*hPer)->ChannelState[map_channel_index(channel)] != CHANNEL_STATE_READY)

相关内容

最新更新

热门标签：