bug en mi codigo

[m@o_614]

Saludos

tengo la siguiente función que se llama checkSumS1() que creo que me esta ocasionando que me truene un programa, no quiero publicar el código completo porque son 3,500 y tantas líneas de código. Lo que hace es que le paso 3 cadenas(Longitud, direccion y codigo_datos) y dentro de la función las concatena en una variable llamada registroS1, después va leyendo registroS1 de dos caracteres en dos. Por ejemplo si tengo ABE31000 primero va a obtener AB, que se supone es un "numero hexadecimal" y lo voy a convertir a su equivalente en decimal, y lo mismo con los demas caracteres. los decimales se van sumando, a la suma se le saca complemento a 1 y se obtienen sus 2 ultimos caracteres

Código (c) [Seleccionar] Expandir

char *CheckSumS1(char *Longitud,char *direccion,char *codigo_datos)
{
   int i,j,k,numero,x = 0,base = 16,suma = 0,complemento;
   char *registroS1,*byte,digito[2],*checksum;
   registroS1 = (char*)malloc(39*sizeof(char));
   checksum = (char*)malloc(3*sizeof(char));
   sprintf(registroS1,"%s%s%s",Longitud,direccion,codigo_datos);
   for(i = 0;registroS1[i];i+=2)
   {
       byte = (char*)calloc(3,sizeof(char));
       for(j = 1,k = i;j <= 2;j++,k++)
       {
           sprintf(digito,"%c",registroS1[k]);
           strcat(byte,digito);
       }
       numero = obtenerNumero(byte,x,base);
       suma+=numero;
       free(byte);
   }
   complemento = ~suma;
   sprintf(checksum,"%02X",complemento);
   checksum = ultimos2bits(checksum);
   return checksum;
}

no se bien cual podria ser la causa de que cada vez que cuando el  codigo llega a esta función para ejecutarla a veces me truena y otras no. no se si  el sprintf y el strcat sean la mejor opcion para usarlos en la función, pero fue lo único que se me ocurrió para que me fuera leyendo de 2 en dos. si alguien me dijera cual es el bug que tiene me codigo se lo agradeceria mucho

gracias

[eferion]

* registroS1 no es necesario que lo crees con memoria dinámica... o al menos, si lo usas con reserva de memoria, que sea para reservar un buffer lo más pequeño posible:

Código (c) [Seleccionar] Expandir

registroS1 = malloc( strlen(Longitud) + strlen(direccion) + strlen(codigo_datos) + 1 );

* digito es un char... no necesitas crear un buffer para ello:

Código (c) [Seleccionar] Expandir

char digito;

// ...

digito = registroS1[ k ];

byte[ j - 1 ] = digito;

* Dado que byte siempre tiene longitud 3 no tiene tampoco mucho sentido que uses memoria dinámica. Si quieres resetear la memoria basta con que uses, por ejemplo, memset:

Código (c) [Seleccionar] Expandir

char byte[ 3 ];
memset( byte, 0, 3 );

* Dado el bucle "for"... j, perfectamente podría ir de 0 a 2, así lo puedes usar de índice para escribir en "byte"

* Estás seguro que la suma de complemento siempre es inferior a 0xFF??? si no es así tienes un desbordamiento de buffer al almacenar el valor en "checksum".

[m@o_614]

gracias eferion por tu respuesta, le haré esos cambios a mi código. Una última pregunta, si tengo declarada un puntero como *cadena y después le asigno memoria así:

codigo_datos = (char*)malloc(33*sizeof(char));

es correcto después de hacer esto inicializarla con NULL??

[eferion]

Si haces:

Código (c) [Seleccionar] Expandir


codigo_datos = (char*)malloc(33*sizeof(char));
codigo_datos = 0;

Estás haciendo que el puntero apunte a otra dirección de memoria, por lo que pierdes la dirección de la memoria reservada y eso resulta en una laguna de memoria.

En cambio, si haces:

Código (c) [Seleccionar] Expandir


codigo_datos = (char*)malloc(33*sizeof(char));
*codigo_datos = 0;

Entonces haces que la primera posición de la memoria reservada valga 0, es decir, contiene una cadena de longitud 0.

Aún así, si lo que quieres es que tener la memoria inicializada al reservarla, puedes usar calloc en vez de malloc. calloc inicializa la memoria, poniendo todos los bytes a 0:

Código (c) [Seleccionar] Expandir

codigo_datos = (char*)calloc(33, sizeof(char));

[rir3760]

no se bien cual podria ser la causa de que cada vez que cuando el  codigo llega a esta función para ejecutarla a veces me truena y otras no.

En mi opinión la causa mas probable es la indicada por eferion, otro escenario donde la función puede reventar se da si el total de caracteres a procesar es impar, en ese caso el bucle principal eventualmente procesara el carácter siguiente al '\0'.

no se si  el sprintf y el strcat sean la mejor opcion para usarlos en la función

Con todo respeto no son, ni de lejos, las mejores opciones. Eso ya lo había comentado en varios de tus temas.

Para el caso es mejor utilizar aritmética de punteros y sscanf para leer cada uno de los bytes (base 16, dos dígitos). Un ejemplo sin las validaciones recomendadas (de malloc y sscanf):

Código (c) [Seleccionar] Expandir

unsigned char chk(char *lon, char *dir, char *cod)
{
   size_t nb = strlen(lon) + strlen(dir) + strlen(cod);
   size_t i;
   
   char *p = malloc(nb + 1);
   unsigned total = 0;
   unsigned byte;
   
   sprintf(p, "%s%s%s", lon, dir, cod);
   for (i = 0; i < nb; i += 2){
      sscanf(p + i, "%2x", &byte);
      total += byte;
   }
   
   return ~total & 0xFF;
}
El tipo de retorno es unsigned char para evitar la reserva de memoria.

Tal vez se pueda mejorar pero ello depende de cierta información que no provees, por ejemplo si el numero de caracteres de cada cadena esta garantizado a ser par se puede evitar la reserva de memoria (mediante malloc) y en su lugar utilizar un array de punteros y dos bucles anidados.

Un saludo

[m@o_614]

muchas gracias por sus respuestas, si el  número de caracteres de la cadena tiene que ser a fuerzas par, porque los voy leyendo de dos en dos