Microsoft 面试问题中此功能的错误是什么？

Question

我在 MS 书面步行采访中被问到这个问题：

在下面的程序中查找错误，该程序应该返回一个带有\n的新字符串。

char* AddnewlinetoString(char *s)
{
  char buffer[1024];
  strcpy(buffer,s);
  buffer[strlen(s)-1] = '\n';
  return buffer;
}

我尝试自己编写代码，并通过将缓冲区变量设为全局变量并使用buffer[strlen(s)] = '\n' 使其工作。但是不知道里面还有很多其他的bug。

Answer 1

我可以看到一些：

未检查输入字符串的长度。

如果strlen(s) > 1023 怎么办？缓冲区中最多可以容纳一个长度为1023 的字符串。

用\n覆盖最后一个字符

您正在用换行符覆盖最后一个字符。你的\n 应该去以前\0 的位置，你需要在\n 之后添加一个新的\0

变量缓冲区对于函数来说是本地的，你正在返回它的地址。

缓冲区的内存在堆栈上分配，一旦函数返回，该内存就会被释放。

我愿意：

char* AddnewlinetoString(char *s) {

  size_t buffLen = strlen(s) + 2; // +1 for '\n' +1 for '\0'
  char *buffer = malloc(buffLen); 
  if(!buffer) {
   fprintf(stderr,"Error allocting\n");
   exit(1);
  }
  strcpy(buffer,s);
  buffer[buffLen-2] = '\n';
  buffer[buffLen-1] = 0;
  return buffer;
}

Answer 2

1. strcpy 没有限制，最好使用 strncpy。
2. 您正在复制到静态缓冲区并返回指针。

Answer 3

这是一个没有错误的 C++ 版本：

std::string AddnewlinetoString(std::string const& s)
{
    return s + "\n";
}

这就是我可能会用 C++0x 编写的方式：

std::string AddnewlinetoString(std::string s)
{
    return std::move(s += "\n");
}

Answer 4

我还要补充一点，方法的名称应该坚持模式，每个单词都应该以大写字母开头：

char* AddNewlineToString(char *s)
{
}

ps。谢谢 Konrad，我已经按照您的建议更改了方法名称

Answer 5

三件事

   int len = strlen(s);
   char* buffer = (char*) malloc (len + 2);   // 1
   strcpy(buffer,s);
   buffer[len] = '\n';           // 2 
   buffer[len+1] = '\0';         // 3
   return buffer;

编辑：基于 cmets

Answer 6

这是一个更正的版本（社区 wiki，以防我遗漏了什么）

// caller must free() returned buffer string!
char* AddnewlinetoString(char *s)
{
  size_t len;
  char * buffer;

  if (s == NULL)
    s = "";

  len = strlen(s);
  buffer = malloc(len+2);
  if (buffer == NULL)
    abort();
  strcpy(buffer,s);
  buffer[len] = '\n';
  buffer[len+1] = 0;
  return buffer;
}

正如 tony 所提到的，s 可能是一个有效的地址，但仍然是一个格式错误的 c 字符串，没有空字节。该函数最终可能会读取，直到它导致段错误或其他一些可怕的事情。虽然这仍然是惯用的 C，但大多数人更喜欢计数字符串（而不是以 null 结尾的字符串。）

// caller must free() returned buffer string!
char* AddnewlinetoStringN(char *s, size_t len)
{
  char * buffer;

  if (s == NULL)
    s = "";

  buffer = malloc(len+1); // only add 1 byte, since there's no need for the nul
  if (buffer == NULL)
    abort();
  strncpy(buffer,s,len);
  buffer[len] = '\n';
  return buffer;
}

Answer 7

这段代码的主要问题是它容易受到stack buffer overflow 的攻击。这是一个经典的例子。

基本上，输入的 char* 可以超过 1024 字节；这些额外的字节将覆盖堆栈，允许攻击者修改函数返回指针以指向他们的恶意代码。然后你的程序会在不知不觉中执行恶意代码。

微软可能会非常关心这些类型的攻击，因为 Code Red Worm 曾在 2001 年使用堆栈缓冲区溢出攻击数十万台运行 IIS Web 服务器软件的计算机。

Answer 8

不需要返回指针。更改传入指针。

int len = strlen(s); s[len] = '\n'; s[len + 1] = '\0';

Answer 9

在 C++ 中应该是

std::string AddNewlineToString(const std::string& s) // pass by const reference
{
    return s + '\n'; // and let optimizer optimize memory allocations
}

Answer 10

使用 strdup 可以非常简单：

char* AddnewlinetoString(char *s) {
char *buffer = strdup(s);
buffer[strlen(s)-1] = '\n';
return buffer;
}

Answer 11

对于 C 风格的字符串，它可能是

char* // we want return a mutable string? OK
AddNewlineToString(
  const char* s // We don't need to change the original string, so it's const.
)
{
     const size_t MAX_SIZE = 1024; // if it's a mutable string,
                                   // it should have a known capacity.

     size_t len = strlen(s);
     if(len + sizeof("\n") // To avoid the magic number "2".
         > MAX_SIZE)
         return NULL; // We don't know what to do with this situation,
                      // the user will check the result and make a decision -
                      // to log, raise exception, exit(), etc.

     // static                    // We want a thread-safe result
     char* buf = new char[1024];  // so we allocate memory in the heap
                                  // and it's C-language-string but not C language :)

     memcpy(buf, s, len); // Copy terminating zero, and rewrite it later? NO.
     memcpy(buf + len, "\n", sizeof("\n")); // The compiler will do it in one action like
        // *(int16_t*)(buf + len) = *(int16_t*)"\n";
        // rather than two assignments.

     return buf;
}

Answer 12

这里有一个更简单的方法：

char* AddnewlinetoString(char *s)
{
  char buffer[strlen(s)+1];
  snprintf(buffer,strlen(s),"%s\n",s);
  return buffer;
}