2008-08-04 23 views
11

He intentado escribir una función de reemplazo de cadena en C, que funciona en un char *, que se ha asignado usando malloc(). Es un poco diferente ya que encontrará y reemplazará cadenas, en lugar de caracteres en la cadena de inicio.¿Por qué recibo un doble error libre con realloc()?

Es trivial si las cadenas de búsqueda y reemplazo tienen la misma longitud (o la cadena de reemplazo es más corta que la cadena de búsqueda), ya que tengo espacio suficiente asignado. Si trato de usar realloc(), aparece un error que me indica que estoy haciendo una doble libre, lo que no veo como soy, ya que solo estoy usando realloc().

Tal vez un poco de código ayudará: trabaja

void strrep(char *input, char *search, char *replace) { 
    int searchLen = strlen(search); 
    int replaceLen = strlen(replace); 
    int delta = replaceLen - searchLen; 
    char *find = input; 

    while (find = strstr(find, search)) { 

     if (delta > 0) { 
      realloc(input, strlen(input) + delta); 
      find = strstr(input, search);    
     } 

     memmove(find + replaceLen, find + searchLen, strlen(input) - (find - input)); 
     memmove(find, replace, replaceLen); 
    } 
} 

El programa, hasta el trato de realloc() en un caso en el que la cadena reemplazado será más larga que la cadena inicial. (Todavía funciona, solo arroja errores y el resultado).

Si ayuda, el código de llamada se parece a:

#include <stdio.h> 
#include <string.h> 
#include <stdlib.h> 

void strrep(char *input, char *search, char *replace); 

int main(void) { 
    char *input = malloc(81); 

    while ((fgets(input, 81, stdin)) != NULL) { 
     strrep(input, "Noel", "Christmas"); 
    } 
} 

Respuesta

11

Como regla general, debe nunca hacer un libre o realloc en un buffer proporcionado por el usuario. No sabe dónde asignó el usuario el espacio (en su módulo, en otra DLL) por lo que no puede usar ninguna de las funciones de asignación en un búfer de usuario.

Siempre que no pueda hacer una reasignación dentro de su función, debe cambiar un poco su comportamiento, como hacer solo un reemplazo, para que el usuario pueda calcular la longitud máxima de cadena resultante y proporcionarle un búfer largo suficiente para que este reemplazo ocurra.

Luego podría crear otra función para hacer los reemplazos múltiples, pero tendrá que asignar todo el espacio para la cadena resultante y copiar la cadena de entrada del usuario. Luego debe proporcionar una forma de eliminar la cadena que asignó.

El resultado es:

void strrep(char *input, char *search, char *replace); 
char* strrepm(char *input, char *search, char *replace); 
void strrepmfree(char *input); 
6

Sólo un tiro en la oscuridad porque no he probado todavía, pero cuando se REALLOC que devuelve el puntero al igual que malloc. Debido a realloc puede mover el puntero, si es necesario lo más probable es operativo en un puntero no válido si no se hace lo siguiente:

input = realloc(input, strlen(input) + delta); 
+0

Y si realloc falla, devuelve NULL y deja solo el búfer existente. Acaba de perder el puntero ... :-( –

4

Nota, intenta modificar su código para deshacerse de los códigos HTML de escape.

Bueno, aunque ha pasado un tiempo desde que usé C/C++, el realloc que crece solo reutiliza el valor del puntero de memoria si hay espacio en la memoria después del bloque original.

Por ejemplo, considere esto:

(xxxxxxxxxx ..........)

Si sus puntero apunta a los primeros x, y. significa la ubicación de la memoria libre, y usted aumenta el tamaño de la memoria apuntada por su variable en 5 bytes, tendrá éxito. Esto es, por supuesto, un ejemplo simplificado ya que los bloques se redondean a un cierto tamaño para la alineación, pero de todos modos.

Sin embargo, si posteriormente intenta crecer otros 10 bytes, y solo hay 5 disponibles, deberá mover el bloque en la memoria y actualizar su puntero.

Sin embargo, en su ejemplo usted está pasando la función un puntero al carácter, no un puntero a su variable, y así mientras la función strrep internamente podría ser capaz de ajustar la variable en uso, es una variable local para la función strrep y su código de llamada quedarán con el valor de la variable del puntero original.

Este valor de puntero, sin embargo, se ha liberado.

En su caso, la entrada es la culpable.

Sin embargo, quisiera hacer otra sugerencia. En su caso, parece que la entrada variable es de hecho entrada, y si lo es, no debe modificarse en absoluto.

Intentaré encontrar otra manera de hacer lo que quiera hacer, sin cambiar la entrada , ya que los efectos secundarios como este pueden ser difíciles de localizar.

0

Mis consejos rápidos.

En lugar de:
void strrep(char *input, char *search, char *replace)
intento:
void strrep(char *&input, char *search, char *replace)

y que en el cuerpo:
input = realloc(input, strlen(input) + delta);

leer general acerca de pasar argumentos de la función como valores/de referencia y realloc() Descripción:)

+0

La notación 'void strrep (char * & input, char * search, char * replace)' no es válida en C, aunque es válida en C++. La pregunta no es , y AFAICT nunca fue etiquetado con C++. En el mejor de los casos, el código debería ser 'void strrep (entrada de char **, búsqueda de char *, reemplazo de char *)', aunque es fácil argumentar que 'char * strrep (const char * input, const char * search, const char * replace) 'es una interfaz viable (las cadenas de entrada no se cambian, la cadena modificada se asigna y se devuelve). –

3

Esto parece funcionar;

char *strrep(char *string, const char *search, const char *replace) { 
    char *p = strstr(string, search); 

    if (p) { 
     int occurrence = p - string; 
     int stringlength = strlen(string); 
     int searchlength = strlen(search); 
     int replacelength = strlen(replace); 

     if (replacelength > searchlength) { 
      string = (char *) realloc(string, strlen(string) 
       + replacelength - searchlength + 1); 
     } 

     if (replacelength != searchlength) { 
      memmove(string + occurrence + replacelength, 
         string + occurrence + searchlength, 
         stringlength - occurrence - searchlength + 1); 
     } 

     strncpy(string + occurrence, replace, replacelength); 
    } 

    return string; 
} 

Suspiro, ¿hay alguna manera de publicar el código sin succión?

+0

Agregando un comentario, ya que el comentario fue escrito como una respuesta, antes de comentar estuvo disponible: Parece que solo cambia la primera vez. ¡Lo cual es probablemente razonable, ya que realmente no dije que debe cambiarlos a todos! –

12

En primer lugar, siento llegar tarde a la fiesta. Esta es mi primera respuesta de stackoverflow. :)

Como se ha señalado, cuando se llama a realloc(), puede cambiar el puntero a la memoria reasignada. Cuando esto sucede, el argumento "cadena" deja de ser válido. Incluso si lo reasigna, el cambio queda fuera del alcance una vez que la función finaliza.

Para responder al OP, realloc() devuelve un puntero a la memoria recién reasignada. El valor de retorno debe almacenarse en algún lugar. En general, usted podría hacer esto:

data *foo = malloc(SIZE * sizeof(data)); 
data *bar = realloc(foo, NEWSIZE * sizeof(data)); 

/* Test bar for safety before blowing away foo */ 
if (bar != NULL) 
{ 
    foo = bar; 
    bar = NULL; 
} 
else 
{ 
    fprintf(stderr, "Crap. Memory error.\n"); 
    free(foo); 
    exit(-1); 
} 

Como TyBoer señala, ustedes no puede cambiar el valor del puntero se pasa como la entrada a esta función. Puede asignar lo que desee, pero el cambio saldrá del alcance al final de la función. En el siguiente bloque, "de entrada" puede o no ser un puntero no válido una vez que la función se completa:

void foobar(char *input, int newlength) 
{ 
    /* Here, I ignore my own advice to save space. Check your return values! */ 
    input = realloc(input, newlength * sizeof(char)); 
} 

Marcos intenta solucionar esta devolviendo el nuevo puntero como la salida de la función. Si lo hace, la persona que llama tiene la responsabilidad de no volver a utilizar el puntero que utilizó para ingresar.Si coincide con el valor de retorno, tiene dos punteros en el mismo lugar y solo necesita llamar a free() en uno de ellos. Si no coinciden, el puntero de entrada ahora apunta a la memoria que puede o no ser propiedad del proceso. La desreferenciación podría causar una falla de segmentación.

Se puede usar un puntero doble para la entrada, así:

void foobar(char **input, int newlength) 
{ 
    *input = realloc(*input, newlength * sizeof(char)); 
} 

Si la persona que llama tiene un duplicado del puntero de entrada en alguna parte, que aún podría duplicar inválida ahora.

Creo que la solución más limpia aquí es evitar el uso de realloc() cuando se intenta modificar la entrada de la persona que llama a la función. Solo malloc() un nuevo búfer, devuélvalo y deje que quien llama decida si desea o no liberar el texto anterior. ¡Esto tiene el beneficio adicional de permitir que la persona que llama conserve la secuencia original!

6

Alguien más se disculpó por llegar tarde a la fiesta, hace dos meses y medio. Oh, bueno, paso bastante tiempo haciendo arqueología de software.

Me interesa que nadie haya comentado explícitamente sobre la pérdida de memoria en el diseño original, o el error "uno por uno". Y fue observar la fuga de memoria que me dice exactamente por qué está obteniendo el error de doble liberación (porque, para ser precisos, está liberando la misma memoria varias veces, y lo está haciendo después de pisotear la memoria ya liberada).

Antes de realizar el análisis, estaré de acuerdo con aquellos que dicen que su interfaz es menos que estelar; sin embargo, si resolvió los problemas de fuga de memoria/pisoteo y documentó el requisito de "memoria asignada", podría ser "OK".

¿Cuáles son los problemas? Bueno, pasas un buffer a realloc(), y realloc() te devuelve un nuevo puntero al área que deberías usar e ignoras ese valor de retorno. En consecuencia, es probable que realloc() haya liberado la memoria original, y luego le pase el mismo puntero nuevamente, y se queja de que está liberando la misma memoria dos veces porque le vuelve a pasar el valor original. Esto no solo filtra la memoria, sino que significa que continúa utilizando el espacio original, y la foto de John Downey en la oscuridad señala que está haciendo un uso incorrecto de realloc(), pero no enfatiza qué tan severamente lo está haciendo. También hay un error "off-by-one" porque no asigna suficiente espacio para el NUL '\ 0' que termina la cadena.

La pérdida de memoria se produce porque no proporciona un mecanismo para decirle a la persona que llama sobre el último valor de la cadena. Debido a que seguiste pisoteando la cadena original más el espacio posterior, parece que el código funcionó, pero si tu código de llamada liberó el espacio, también obtendría un doble error libre, o podría obtener un volcado principal o equivalente porque la información de control de memoria está completamente codificada.

Su código tampoco protege contra el crecimiento indefinido; considere reemplazar 'Noel' por 'Joyeux Noel'. Cada vez, agregaría 7 caracteres, pero encontraría otro Noel en el texto reemplazado y lo expandiría, y así sucesivamente. Mi corrección (a continuación) no resuelve este problema; la solución simple es, probablemente, verificar si la cadena de búsqueda aparece en la cadena de reemplazo; una alternativa es omitir la cadena de reemplazo y continuar la búsqueda después de ella. El segundo tiene algunos problemas de codificación no triviales para abordar.

lo tanto, mi revisión sugerida de la función llamada es:

char *strrep(char *input, char *search, char *replace) { 
    int searchLen = strlen(search); 
    int replaceLen = strlen(replace); 
    int delta = replaceLen - searchLen; 
    char *find = input; 

    while ((find = strstr(find, search)) != 0) { 
     if (delta > 0) { 
      input = realloc(input, strlen(input) + delta + 1); 
      find = strstr(input, search);    
     } 

     memmove(find + replaceLen, find + searchLen, strlen(input) + 1 - (find - input)); 
     memmove(find, replace, replaceLen); 
    } 

    return(input); 
} 

Este código no detecta errores de asignación de memoria - y probablemente se estrella (pero si no es así, pérdidas de memoria) si realloc() falla. Consulte el libro de Steve Maguire "Escribir código sólido" para una extensa discusión sobre los problemas de gestión de la memoria.

+1

Gracias, este es un muy buen análisis de lo que estaba haciendo mal (y que el doblemente libre era, en cierto sentido, un subproducto de varias cosas que estaba haciendo mal). Creo que tenía en mente que realloc() acaba de extender la asignación de memoria, lo cual no tiene ningún sentido cuando lo pienso. –

3

realloc es extraño, complicado y solo se debe usar cuando se trata de mucha cantidad de memoria muchas veces por segundo. es decir, donde realmente hace que su código sea más rápido.

he visto código donde se utilizó y trabajó para cambiar el tamaño de la memoria intermedia

realloc(bytes, smallerSize); 

, por lo que es más pequeño. Trabajó alrededor de un millón de veces, luego, por alguna razón, realloc decidió que, incluso si estuviera acortando el búfer, le daría una buena copia nueva. Así que chocas en un lugar aleatorio 1/2 segundo después de que sucedieron las cosas malas.

Utilice siempre el valor de retorno de realloc.

Cuestiones relacionadas