Problema con lista enlazada en c

[eferion]

Empecemos:

Código C:

int insertIntoList(list *lst,int pos, void *data){
  // ... 
  if((lst = (list*)malloc(sizeof(list)))==NULL)
 

Si resulta que la función me crea una lista SIEMPRE... ¿cómo se supone que voy a añadir un segundo elemento a la lista? La segunda vez que llame a la función el segundo elemento lo añadirá en una segunda lista.

lst es un puntero, eso quiere decir que cualquier cambio que realice sobre la memoria a la que apunta se verán reflejados fuera de la función... ahora, si yo modifico la posición de memoria a la que apunta el puntero, dicho cambio es local, por lo que fuera de la función no tendrá efecto.

Código Ejemplo:

Si tengo:
list* lista = 0x1000;
insertIntoList(lista,0,algo);
 
entonces lst apuntará a 0x1000 y todos los cambios que realice sobre lst se almacenarán en 0x1000, pero...
si dentro de la función pongo lst=0x2000, todos los cambios que realice a partir de ese momento en lst
se almacenarán en 0x2000. Fuera de la función, lista sigue apuntando a 0x1000, por lo que no podrá ver los
cambios que se han realizado en 0x2000.
 

Código C:

if ((new_data->data = (char *) malloc (50 * sizeof (char)))== NULL)
 

¿Qué pasa si quiero almacenar una cadena de 100 caracteres? Pasa que voy a pisar memoria que no me pertenece. Este efecto se llama "buffer overflow" y es muy peligroso
¿Qué pasa si quiero almacenar una cadena de 2 caracteres? Estoy desaprovechando un montón de memoria. Por qué no puedes, simplemente, reservar la memoria mínima necesaria para almacenar la cadena? Usa strlen para conocer el tamaño de la cadena y haz una reserva de memoria acorde a esos requisitos (recuerda que necesitas reservar un byte para el caracter nulo de fin de cadena).

Código C:

for (i = 1; i < pos; ++i)
    actual = actual->next;
 

¿Y si pos es mayor que el número de elementos de la lista? Pues sucederá que acabarás pisando memoria que seguramente no te pertenezca y eso provocará que la aplicación te premie con un pantallazo. En sistemas operativos antiguos puedes incluso conseguir bloquear el sistema.

Si tienes lst->length, ¿Por qué motivo no compruebas que pos sea menor que el tamaño de la lista? También puedes optar por comprobar que actual->next no sea NULL antes de pasar al siguiente elemento.

Código C:

if (actual->next == NULL)
    return -1;
 

Al llegar aquí me vienen a la mente unas cuantas dudas:

• ¿Qué pasa si intento añadir un elemento al final de la lista? Sucederá que actual->next valdrá NULL, por lo que no podré añadir un elemento a la lista...
• ¿Qué sucede si la lista está vacía? sucederá que actual, que es igual a lst->first vale NULL, ¿qué me devolverá NULL->next?
• ¿Y si quiero añadir un elemento al inicio de la lista? ¿No se debería modificar lst->first?
• Después de ese return, la memoria reservada con anterioridad (lst,new_data y new_data->data se pierde. No es que se libere, se queda reservada pero ya no tienes ninguna referencia que te permita llegar hasta a ella, por lo que no puedes liberarla ni hacer nada util con ella. Se dice entonces que tu programa tiene fugas de memoria y ese es un asunto bastante serio. Prueba a llamar a esta función un millón de veces y mira como aumenta el consumo de memoria de tu aplicación... Esto es aplicable a dos de los tres return que tienes antes que este, lo he puesto aquí para no repetirme.

Código C:

strcpy (new_data->data, data);
 

Si resulta que data va a almacenar cadenas de caracteres... ¿por qué no se declara como char* y así ya queda clara su finalidad?
Manipular un puntero genérico void* como si de una cadena se tratase es peligroso porque puede almacenar información binaria, la cual puede contener un número indeterminado de '\0' que forman parte del contenido de la variable.

Código C:

int listLength(list *lst){
  node *actual;
  actual = lst->first;
 
  while (actual != NULL){
      printf ("%p - %s\n", actual, actual->data);
      actual = actual->next;
}}
 

Esto... si yo me encuentro una función con nombre listLength yo esperaría que la función me devolviese el número de elementos de la lista. De primeras esta función no tendría sentido, ya que ya existe un lista->length, pero bueno. Lo que pasa es que esta función no solo no realiza la tarea que aparenta sino que intenta volcar el contenido de la lista en la pantalla. ¿Seguro que esto va aquí?

Por otro lado, si una función tiene un tipo de retorno diferente de void se hace necesario incluir al menos un return. No hay que olvidar que los warning del compilador son en realidad errores, lo que sucede es que estos errores no impiden que el código compile... pero son errores. Son errores que pueden provocar problemas de cualquier tipo, incluso comportamientos erráticos que dependerán del compilador utilizado.

Con esto creo que tienes entretenimiento para un rato :)

Un saludo.