这是宏滥用吗?

DCD_BADWRITE 如果 X 等于-1。然而,所讨论的宏从 功能。例子:

int foobar(int value) {
    CHECK_FREAD(value, "Whatever");
    return 0;
}

将扩展到:

int foobar(int value) {
    if (value == -1) {
        return (DCD_BADWRITE);
    };
    return 0;
}

然而,如果它是一个函数,那么外部函数( foobar )很乐意忽略结果并返回0。

一个看起来很像函数的宏,但在一个关键的方面表现出不同的行为,在我看来是一个主要的否定 是 宏滥用。

return 是从调用函数返回的,而不是宏中的块。

这类东西给宏起了一个坏名字,因为它隐藏了一个重要的控制逻辑。

我对是否将这种宏称为“滥用”持怀疑态度,但我要说,这不是一个好主意。

return 在宏中。如果不使调用它的函数的控制流变得笨拙或难以理解,就很难创建一个正确处理返回的宏。你肯定不想在没有意识到的情况下“意外地”从函数返回。这可能是恼人的调试。

if (some_condition)
    CHECK_FREAD(val, "string")
else
    something_else();

不会做你认为会做的事 else if 宏内部)。宏需要包含在 { }

另外,第二个参数没有使用。这里明显的问题是实现与文档不匹配。隐藏的问题是当宏被这样调用时:

char* messages[4];          // Array of output messages
char* pmsg = &messages[0];
....
CHECK_FREAD(val, pmsg++);

为什么不编写一个函数来封装 read 错误检查呢?它可以在成功时返回零,也可以返回错误代码。如果错误处理代码在所有

int your_function(whatever) {
    int ret;
    ...

    if ((ret=read_helper(fd, &input_integer)) != 0)
        goto error_case;

    ...

    // Normal return from the function
    return 0;

error_case:
    print_error_message_for_code(ret);
    return ret;
}

只要你所有的电话 read_helper 将其返回值赋给 ret ,您可以在整个函数中重复使用相同的错误处理代码块。你的职能 print_error_message_for_code switch

我承认很多人害怕 goto ,但是重新使用公共错误处理代码而不是一系列嵌套的块和条件变量可以是一个合适的用例。只要保持它的干净和可读性(每个函数一个标签,并保持错误处理代码简单)。

这是否是虐待是一个品味的问题。但我看到了一些主要的问题

1. 文件是完整的 错误的
2. 执行是非常危险的 if 以及一个可能的 晃来晃去的 else
3. 命名并不意味着这是真的 周边功能

所以这绝对是非常糟糕的风格。

如果宏在一个源文件中,并且只在该文件中使用,那么我发现它比在某个头文件中关闭时要不那么令人讨厌。但我不太喜欢返回的宏(尤其是 终止应用程序,并实际返回),更不用说有条件地返回,因为这样很容易造成以下内存泄漏:

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

要执行宏实际执行的操作,我更希望完全跳过宏,然后编写:

if (retval == -1) return DCD_BADREAD;

这并不是一个很长的代码行,而且将它的不同实例共享也没有什么真正的好处。认为通过封装/隐藏现有的、有充分文档记录的方法来改进事情也可能不是一个好主意 read 表示错误,这是C程序员广泛理解的。如果没有别的,这个检查完全忽略了一个事实 阅读 可以产生比你要求的更少的数据,没有明显的原因。这不是宏的直接错误,但是宏背后的整个想法似乎来自错误检查。

控制流的宏 可以 crReturn 至少 好像要回来了。

我认为这个宏滥用的例子有两个原因:(1)宏的名称没有明确说明它做了什么,最明显的是它可能返回;(2)宏在语法上不等同于一个语句。代码类似

  if (someCondition)
    CHECK_FREAD(whatever);
  else
    do_something_else();

会失败的。我的首选更改:

#define LOG_AND_RET_ERROR(msg) do {LOG_ERROR(msg); return DCD_BADREAD;} while(0)
  if (x==-1) LOG_AND_RET_ERROR(msg);

好吧,如果你定义了一个短函数,那么你必须在每个调用站点输入更多的字符。即。

CHECK_FREAD(X, msg)

与。

if (check_fread(X, msg)) return DCD_BADREAD;

最让我担心的是,一些新引入的开发人员可能会看到这一点,并考虑这样做

if (CHECK_FREAD(...)) {
   /* do stuff here */
} else {
   /* error handle */
}

这显然是错误的。

而且,似乎 msg DCD_BADREAD 使情况恶化。。。