这是不好的编码做法吗？

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指针，并且您在执行指针算术和解引用之前没有检查它。

您可能想做类似的事情：

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()调用-该参数的作用是指定目标缓冲区的大小，以免溢出。您传递的值毫无意义，皮棉也可能对此抱怨。您可能是指end-(start-1)，可能更简单地说是strlen(start)+1。

无论如何，即使将strlen(start)+1作为最后一个参数传递给strlcpy()，也违反了该参数的意图，并消除了应该提供的安全性strlcpy()。您可能还只是使用了strcpy(Fp_smsSender,start)-如果您不知道Fp_smsSender目标缓冲区有多大，则应该准确地做到这一点（或进行修复，以便知道缓冲区有多大）。将更清楚代码实际上在做什么。

Answer 2

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

[第二个（更重要的是），您传递给strlcpy的长度是错误的：您想要end - start或类似的东西（您已将其取反），但更重要的是，strlcpy的length参数应为destination缓冲区的大小，而不是源字符串的大小。