Problema al liberar memoria con free()

[mester]

Hola.
Estoy haciendo una shell y de momento va bien xd el problema surge cuando quiero liberar la variable que contiene los argumentos. Adjunto codigo:

Código (c) [Seleccionar] Expandir


char **GetCommand(int maxLength) { /*Recibe un comando*/
  int argc, secArgc;
  char *stdBuffer = (char *)malloc(sizeof(char) * maxLength);
  char **stdCommand = (char **)malloc(sizeof(char *) * (maxLength / 2));

  argc = secArgc = 0;
  stdCommand[argc] = (char *)malloc(sizeof(char) * (maxLength / 4));

  printf("%s ", COMMAND_PROMPT);
  fgets(stdBuffer, maxLength, stdin);
  stdBuffer[strlen(stdBuffer) - 1] = '\0';

  while(*stdBuffer) {
    if(*stdBuffer == '\n') {
      break;
    }

    if(*stdBuffer == ' ') {
      secArgc = 0;
      stdCommand[++argc] = (char *)malloc(sizeof(char) * (maxLength / 4));
    }

    stdCommand[argc][secArgc++] = *stdBuffer++;
  }
  stdBuffer = NULL;
  free(stdBuffer);

return stdCommand;
}

Con esta función se recibe un comando. Y este sería el main:

Código (c) [Seleccionar] Expandir


char **stdLine = NULL;

  while(stdLine = GetCommand(1024)) { /*Mientras se reciba un comando*/
    printf("%s\n", stdLine[0]);

    switch(comExecute((const char **)stdLine)) {
      case 0: break;
      case 1: perror("Can\'t execute these command\n");
      case EXIT_COMMAND: exit = 1;
    }
    if(exit) { /*Si ha escrito 'exit'*/
      break;
    }

    free(stdLine);
  }

El problema surge con el free(stdLine); que a la hora de imprimirlo se ve que se llena de basura o algo así, porque cuando se ejecuta el printf del main me saca carácteres raros al final de cadena que impiden que sea bien leida. He pensado en poner stdLine = NULL; pero lo que quiero es liberar memoria. ¿Sería lo mismo poner free(stdLine) que stdLine = NULL? Yo creo que no, pero como no me dedico a la programación profesionalmente, sino más por afición, pues no tengo mucha idea.

Gracias de antemano.

[AlbertoBSD]

trata de usar calloc en lugar de malloc malloc te da memoria no inicializada en 0 y calloc si lo hace.

Trata de siempre pedir un byte mas de lo que necesitas para dejar un byte nullo al final.

Asi quedaria

Código (c) [Seleccionar] Expandir

  char *stdBuffer = (char *)calloc(sizeof(char) * maxLength+1,1);
  char **stdCommand = (char **)calloc(sizeof(char *) * (maxLength / 2)+1,1);

Veo que estas separando por cada espacio por que no usas strtok?

Un ejemplo seria el que mostre en el siguiente tema.

https://foro.elhacker.net/programacion_cc/mi_programa_crashea_al_usar_strtok-t452597.0.html

te dejo un video que he hecho para que veas la diferencia.

[youtube=640,360]https://www.youtube.com/watch?v=iBf7AThP1w8[/youtube]

saludos

[mester]

Veo que estas separando por cada espacio por que no usas strtok?

Porque prefiero usar funciones mías. Una cosa, es lo mismo si hago esto:

Código (c) [Seleccionar] Expandir


free(stdLine);
Que si hago esto:

Código (c) [Seleccionar] Expandir


for(i = 0; i < 10; i++) {
  free(stdLine[i]);
}
free(stdLine);

Y otra cosa... ¿Como puedo saber si un puntero está apuntando a alguna direccion de memoria?
Porque por ejemplo puedo declararlo pero no inicializarlo, y despues al liberarlo me da una violacion de segmento.

Gracias.

[AlbertoBSD]

[

Porque prefiero usar funciones mías. Una cosa, es lo mismo si hago esto:


Y otra cosa... ¿Como puedo saber si un puntero está apuntando a alguna direccion de memoria?
Porque por ejemplo puedo declararlo pero no inicializarlo, y despues al liberarlo me da una violacion de segmento.

Gracias.

Yo tambien prefiero usar funciones propias para aprender los algoritmos y en general saber lo que esta haciendo el codigo.

Tienes que hacer una validaciones..

Código (c) [Seleccionar] Expandir


char *stdBuffer = NULL;   //Asignacion redundante pero ña agrego para fines didacticos...
stdBuffer = calloc(maxLength+1,1);
if(stdBuffer != NULL){
//guardar cosas en stdBuffer
...
free(stdBuffer);
}
else{
//No se pudo inicializar error... muy raro pero puede pasar
}


Código (c) [Seleccionar] Expandir


for(i = 0; i < 10; i++) {
 if(stdLine[i] != NULL){
   free(stdLine[i]);
   stdLine[i] = NULL;
 }
}
if(stdLine != NULL){
 free(stdLine);
 stdLine = NULL;
{


Este es el correcto... tienes que liberar primero los apuntadores individuales y despues el doble apuntador.. y validar antes que no sean NULL.

ahora tambien ten cuidado de no liberar 2 veces un mismo apuntador ahi marcaria error..

[mester]

Vale, ya me ha quedado claro, gracias por la ayuda, camarada.

[ivancea96]

Código (c) [Seleccionar] Expandir

char **GetCommand(int maxLength) { /*Recibe un comando*/
int argc, secArgc;
char *stdBuffer = (char *)malloc(sizeof(char) * maxLength);
char **stdCommand = (char **)malloc(sizeof(char *) * (maxLength / 2));

argc = secArgc = 0;
stdCommand[argc] = (char *)malloc(sizeof(char) * (maxLength / 4));

fgets(stdBuffer, maxLength, stdin);

while(*stdBuffer) {
if(*stdBuffer == '\n') {
break;
}

if(*stdBuffer == ' ') {
stdCommand[argc][secArgc] = '\0';
stdCommand[++argc] = (char *)malloc(sizeof(char) * (maxLength / 4));
secArgc = 0;
}else
stdCommand[argc][secArgc++] = *stdBuffer;
stdBuffer++;
}
stdCommand[argc][secArgc] = '\0';
free(stdBuffer);
stdBuffer = NULL;

return stdCommand;
}

Lo ideal, es que vayas poniendo un '\0' al final de cada cadena.
Es interesante ver que no tienes forma de saber el numero de argumentos del comando. Tendrás que devolver tambien un entero.
En cuanto al código, reorganicé para 2 cosas:
-1: poner los '\0'
-2: no anexar el espacio a las cadenas. Para eso el 'else'.

Por lo demás, estaría bien que primero calculases la cantidad de argumentos que tiene la cadena, y luego hicieras los malloc. Sinó, tendrás un montón de memoria perdida.

[mester]

En cuanto al código, reorganicé para 2 cosas:
-1: poner los '\0'
-2: no anexar el espacio a las cadenas. Para eso el 'else'.

Por lo demás, estaría bien que primero calculases la cantidad de argumentos que tiene la cadena, y luego hicieras los malloc. Sinó, tendrás un montón de memoria perdida.

siii, eso ya lo tenía puesto en el codigo, es que este no lo he actualizado. A ver que te parece este:

Código (c) [Seleccionar] Expandir


char **GetCommand(int maxLength, int *numArgs, int *background) { /*Recibe un comando*/
 int argc, secArgc;
 char *stdBuffer = (char *)calloc(sizeof(char) * maxLength, sizeof(char) + 1);
 char **stdCommand = (char **)calloc(sizeof(char *) * (maxLength / 2) + 1, sizeof(char));

 argc = secArgc = *background = 0;
 stdCommand[argc] = (char *)calloc(sizeof(char) * (maxLength / 6), sizeof(char));

 printf("%s ", COMMAND_PROMPT);
 fgets(stdBuffer, maxLength, stdin);
 stdBuffer[strlen(stdBuffer) - 1] = '\0';

 if(!seastr("&", stdBuffer)) {
   *background = 1;
 }

 while(*stdBuffer) {
   if(*stdBuffer == '\n') {
     break;
   }

   if(*stdBuffer == ' ') {
     *stdBuffer++;
     if(*stdBuffer != '&') {
       stdBuffer[++secArgc] = '\0';
       secArgc = 0;
       stdCommand[++argc] = (char *)calloc(sizeof(char) * (maxLength / 4), sizeof(char));
     }
   }
   else {
     stdCommand[argc][secArgc++] = *stdBuffer++;
   }
 }
 *numArgs = argc;

 stdCommand[++argc] = NULL;
 stdBuffer = NULL;
 free(stdBuffer);

return stdCommand;
}

Pongo lo del & porque al ser una shell, el & indica que el proceso será ejecutado en background y tal. Y en cuanto a la funcion seastr() es una funcion propia que busca en una cadena y devuelve ciertos valores:

Código (c) [Seleccionar] Expandir


int seastr(const char *str1 /*word*/,
            const char *str2 /*string*/) {
/*Returns 0 if str1 is in str2*/
  if(str1 == NULL||str2 == NULL) {
    return 1;
  }

  char *cp = (char *)str2;
  char *sr, *fr;

  while(*cp) {
    sr = cp;
    fr = (char *)str1;
    while(*sr && *fr && (*sr == *fr)) {
      sr++, fr++;
    }
    if(!*fr) {
      return 0;
    }
    cp++;
  }
return 1;
}

Si ves algo que se podría mejorar de ahí, por favor, dimelo. Gracias.

[AlbertoBSD]

Todo bien 

Solo como comentario...

Código (c) [Seleccionar] Expandir

  char *stdBuffer = (char *)calloc(maxLength+ 1, sizeof(char));
  char **stdCommand = (char **)calloc( (maxLength / 2) + 1, sizeof(char*));

el sizeof(char) en el primer parametro esta sobrado por que se especifica en el segundo parametro...

Ahora el segundo calloc estas reservando memoria para X cantidad de apuntadores entonces el sizeog(char) le falta el *  dentro del parentesis para que te reserve elementos del damañao de un apuntador usualemnte son elementos de 4 u 8 bytes..

Sobre la variable maxLength no estoy seguro de cuanto valga pero si alguien mete mas comandos que maxLength  puede hacer tu app vulnerable a buffer overflow....

Pero solo es una observacion para alguien que esta aprendiendo por hobby te tengo que decir que dominas el lenguaje con cierta maestria...

Saludos!