trabajo - ¿Cuáles son los errores en esta función de una pregunta de entrevista de Microsoft?

preguntas y respuestas en una entrevista de trabajo para un banco

Question

Me hicieron esta pregunta en la entrevista escrita de MS:

Encuentre errores en el programa a continuación, que se supone que devuelve una nueva cadena con /n anexado a él.

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

Intenté codificarme y pude hacer que funcionara al hacer que la variable del búfer fuera global y tener buffer[strlen(s)] = ''/n'' . Pero no sabía que había muchos otros errores en él.

Answer 1

1. no hay límite en strcpy, mejor use strncpy.
2. está copiando a un búfer estático y devolviendo el puntero.

Answer 2

3 cosas

   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;

Edición: Basado en comentarios.

Answer 3

Aquí hay una versión corregida (wiki de la comunidad en caso de que me haya perdido algo)

// 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;
}

Como lo menciona Tony, s puede ser una dirección válida pero aún así puede ser una cadena C mal formada, sin bytes nulos. La función podría terminar leyendo hasta que cause una falla de seguridad, o alguna otra cosa horrible. Si bien esto sigue siendo una C idiomática, la mayoría de las personas prefieren cadenas contadas (en lugar de las terminadas en nulo).

// 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 4

Aquí hay una versión de C ++ sin errores:

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

Y aquí es cómo probablemente escribiría eso en C ++ 0x:

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

Answer 5

Aquí va una forma más sencilla:

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

Answer 6

El principal problema con este código es que es vulnerable a una explotación de desbordamiento de búfer de pila . Es un ejemplo clásico.

Básicamente, la entrada char * puede tener más de 1024 bytes; estos bytes adicionales luego sobrescribirán la pila, permitiendo que un atacante modifique el puntero de retorno de la función para que apunte a su código malicioso. Su programa luego ejecutará involuntariamente el código malicioso.

Podría esperarse que Microsoft se preocupe mucho por este tipo de explotaciones, ya que el Code Red Worm utilizó un desbordamiento de búfer de pila para atacar a cientos de miles de computadoras que ejecutan software de servidor web IIS en 2001.

Answer 7

En C ++ debería ser

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

Answer 8

No hay necesidad de puntero de retorno. Cambia el puntero entrante.

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

Answer 9

Para cuerdas de estilo C puede ser

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 10

Puedo ver algunos:

La longitud de la cadena de entrada no está marcada.

¿Qué pasa si el strlen(s) > 1023 ? Puede ajustar una cadena de longitud a lo sumo 1023 en el búfer.

Sobrescribiendo el último char con /n

Usted está sobreescribiendo el último char con nueva línea. Tu /n debería ir donde solía estar /0 y debes agregar un nuevo /0 después de /n

El búfer variable es local para funcionar y está devolviendo su dirección.

La memoria para el búfer se asigna en la pila y una vez que la función regresa, esa memoria se libera.

Yo lo haría:

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 11

Se puede hacer muy simple usando strdup:

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

Answer 12

También agregaría que el nombre del método debe seguir el patrón y cada palabra debe comenzar con mayúscula:

char* AddNewlineToString(char *s)
{
}

PD. Gracias Konrad, he cambiado el nombre del método como sugeriste