这是不好的编码习惯吗？

Question

我正在使用 PC-lint 来分析我的代码，这些行产生了几个错误。这让我想知道我的编码实践是否错误？

char *start;
char *end;

// Extract the phone number
start = (char*) (strchr(data, '\"') +1);
end = (char*) strchr(start, '\"');
*end = 0;
strlcpy((char*)Fp_smsSender, start , start-(end-1));

编辑： 经过你的帮助，我现在有了：

char *start;
char *end;

if (data != NULL)
{
  // Extract the phone number
  start = strchr(data, '\"');
  if (start != NULL)
  {
    ++start;
    end = strchr(start, '\"');

    if (end != NULL)
    {
      *end = 0;
      strlcpy((char*)Fp_smsSender, start , FP_MAX_PHONE);
    }
  }

看起来怎么样？

Answer 1

有两件事：首先，您不处理 strchr 的 NULL 返回。

其次（更严重的是），您传递给 strlcpy 的长度是错误的：您需要 end - start 或类似的东西（您已经颠倒了），但更根本的是， strlcpy 的长度参数应该是目标缓冲区的大小，而不是源字符串。

Answer 2

我想 lint 抱怨的是 strchr() 可能会返回 NULL 指针，而您在执行指针算术并取消引用它之前没有检查它。

您可能想做类似的事情：

char *start;
char *end;

// Extract the phone number
start = strchr(data, '\"');
if (!start) handle_error();

++start; // skip the '\"'
end = strchr(start, '\"');
if (!end) handle_error();

*end = 0; 
strlcpy((char*)Fp_smsSender, start, size_of_Fp_smsSender_buffer);

请注意，我更改了 strlcpy() 调用的最后一个参数 - 该参数的用途是指定目标缓冲区的大小，这样就不会溢出它。你传递的值根本没有意义，lint 也可能抱怨这一点。您可能指的是 end-(start-1)，它可能更简单地表述为 strlen(start)+1。

无论如何，即使将 strlen(start)+1 作为最后一个参数传递给 strlcpy() 也会违反参数的意图，并删除安全 strlcpy() 应该提供。您不妨简单地使用 strcpy(Fp_smsSender,start) - 如果您不知道 Fp_smsSender 目标缓冲区有多大，您应该这样做（或者修复一些问题，这样你就知道缓冲区有多大）。代码实际上在做什么会更清楚。