He detectado el siguiente fragmento de código C , marcado como MALO (también conocido como desbordamiento de búfer incorrecto). El problema es que no entiendo muy bien por qué. La longitud de la cadena de entrada se captura antes de la asignación, etc.
char *my_strdup(const char *s) { size_t len = strlen(s) + 1; char *c = malloc(len); if (c) { strcpy(c, s); // BAD } return c; }Actualización de los comentarios:
+1 después de la llamada strlen() para asignar de forma segura el espacio en el montón que también mantendrá el terminador de cadena ( '\0' )No hay ningún error en su función de muestra.
Sin embargo, para que sea obvio para los futuros lectores (tanto humanos como mecánicos) que no hay ningún error, debe reemplazar la llamada strcpy con memcpy :
char *my_strdup(const char *s) { size_t len = strlen(s) + 1; char *c = malloc(len); if (c) { memcpy(c, s, len); } return c; } De cualquier manera, se asignan len bytes y se copian len bytes, pero con memcpy ese hecho se destaca mucho más claramente para el lector.
No hay problema con este código.
Si bien es posible que strcpy pueda causar un comportamiento indefinido si el búfer de destino no es lo suficientemente grande para contener la cadena en cuestión, el búfer se asigna para que tenga el tamaño correcto. Esto significa que no hay riesgo de sobrepasar el búfer.
Es posible que vea que algunas guías recomiendan usar strncpy en su lugar, lo que le permite especificar la cantidad máxima de caracteres para copiar, pero esto tiene sus propios problemas. Si la cadena de origen es demasiado larga, solo se copiará el número especificado de caracteres; sin embargo, esto también significa que la cadena no termina en nulo, lo que requiere que el usuario lo haga manualmente. Por ejemplo:
char src[] = "test data"; char dest[5]; strncpy(dest, src, sizeof dest); // dest holds "test " with no null terminator dest[sizeof(dest) - 1] = 0; // manually null terminate, dest holds "test" Me inclino por el uso de strcpy si sé que la cadena de origen encajará; de lo contrario, strncpy y anularé manualmente.
No puedo ver ningún problema con el código cuando se trata del uso de strcpy
Pero debe tener en cuenta que requiere que s sea una cadena C válida. Es un requisito razonable, pero debe especificarse.
Si lo desea, puede marcar NULL simplemente, pero diría que está bien prescindir de él. Si está a punto de hacer una copia de una "cadena" apuntada por un puntero nulo, entonces probablemente debería verificar el argumento o el resultado. Pero si quieres, solo agrega esto como la primera línea:
if(!s) return NULL;Pero como dije, no agrega mucho. Simplemente hace posible cambiar
if(!str) { // Handle error } else { new_str = my_strdup(str); }para:
new_str = my_strdup(str); if(!new_str) { // Handle error }No es realmente una gran ganancia