¿Alguna sugerencia mejor para este c funciones copyString, concatString

Me pidieron que hiciera 2 funciones, copyString y concatString, las hice e implementé ambas, pero en la salida que obtuve, me dijeron que se puede hacer mejor, pero nunca se me explicó cómo.
ahora me está matando, ¿qué podría hacer mejor, así que aquí está el código y estaré encantado de escuchar cualquier sugerencia?

void copyString (char **strDst, const char *strSrc) { char *strTmp = NULL; int length = strlen (src); if (*strDst== NULL) { *strDst= malloc (length); } else { if (strlen(*strDst) != length) { strTmp = *strDst; } *strDst= malloc (length); } strcpy (*strDst, strSrc); if (strTmp != NULL) free (strTmp ); } void concatString (char **strDst, const char *cat) { int cat_length = strlen (cat); if (cat_length > 0) { *strDst= realloc (*strDst, strlen (*strDst) + cat_length); strcat (*strDst, cat); } } void main(int argc, char *argv[]) { char *str = NULL; copyString(&str, "Hello World"); puts(str); copyString(&str,str+6); puts(str); concatString(&str, " Pesron"); } 

La salida debe ser como sigue:
1.Hola mundo
2. mundo
3. Persona del mundo

Gracias.

Errores:

strlen devuelve la longitud excluyendo el terminador nulo, por lo que todos los tamaños que asigna son demasiado pequeños.

En el caso en el que if (strlen(*strDst) != length) es falso, (es decir, las longitudes son iguales), se filtra el búfer anterior.

realloc y malloc pueden fallar, deberías poder escribir código para hacer frente a eso.

La forma correcta de usar realloc es:

 char *newbuf = realloc(oldbuf, newsize); if (newbuf == NULL) { // handle the error somehow, and note that oldbuf is still allocated } else { oldbuf = newbuf; } 

“Manejar el error de alguna manera” puede requerir decidir qué hacer, dependiendo de lo que la documentación de sus dos funciones diga que hacen en caso de falla. Si no lo dice entonces debería.

No se garantiza que (Picky) int sea ​​un tipo lo suficientemente grande como para mantener la longitud de una cadena. Use size_t (a menos que tal vez haya sido estrictamente prohibido usar tipos sin firma, en cuyo caso hay ssize_t ).

Cosas que puedes mejorar:

No es necesario utilizar strTmp la manera que lo hace, puede liberar la cadena inmediatamente en lugar de al final de la función. [Editar: sí, hay una necesidad, parece haber un requisito de que copyString pero no concatString debería permitir la superposición de origen y destino. Personalmente todavía lo escribiría de forma ligeramente diferente.]

En if (strTmp != NULL) free (strTmp ); la prueba es redundante ya que es válido para llamar free con un puntero nulo, y hacerlo no tiene efecto.

Usted hace *strDst= malloc (length); En ambos casos en copyString .

Fugas main memoria ya que nunca libera str .

main debe devolver int , no void .

Así es como podría escribirlos:

Dado que no puede cambiar el código de llamada para hacer que se compruebe si hay errores, debe abort() o escribir algo allí que pueda llamar. Dado que la función main se escribió bajo el supuesto de que las llamadas no pueden fallar, abort() es probablemente la solución menos mala.

Probablemente sería mejor para la persona que llama si las funciones devuelven un valor que indica éxito o fracaso, pero estamos limitados por el código de llamada existente. Para ser honesto, esa no es una situación totalmente irreal para ser progtwigda para …

 void concatString (char **strDst, const char *cat) { size_t dstlen = *strDst ? strlen(*strDst) : 0; char *buf = realloc(*strDst, dstlen + strlen(cat) + 1); if (!buf) { abort(); } strcpy(buf + dstlen, cat); *strDst = buf; } void copyString (char **strDst, const char *strSrc) { char *buf = malloc(strlen(strSrc) + 1); if (!buf) { abort(); } strcpy(buf, strSrc); free(*strDst); *strDst = buf; } 

Además de lo que Steve Jessop menciona en su respuesta, no hay errores en sus fonts, pero faltan:

  • validación de parámetros de entrada
  • devuelva los errores a través del valor de error (por ejemplo, como un código de retorno entero de la función, en lugar de void