[C con Clase] ayuda por favor

[Davidson, Steven]

Hola Marcela,

Veo que Salvador se me ha adelantado, pero te doy mi análisis del problema
que llevaba escribiendo :)

2013/2/15 Marcela Bustamante <marcela_bustamante en itrsa.com.ar>

> Buenas tardes, este ejercicio me está presentando el error segmentation
> fault y la verdad que no tengo idea de que estoy haciendo mal desde ya
> muchas gracias por la ayuda o información que me puedan brindar****
>
> **
>
Veamos el programa. Te voy comentando a medida que vaya viendo el código
fuente.

>  **
>
> /*Ejercicio 8.2.****
>
> Repetir el ejercicio anterior creando una estructura struct dNode para una
> lista doblemente****
>
> enlazada.*/****
>
> ** **
>
> #include <stdio.h>****
>
> #include <stdlib.h>****
>
> **
>

Te falta incluir <string.h>, porque usas la función 'strncmp()' en el
programa.

**
>
> //declaro la estructura la cual va a ser un tipo de dato que contiene
> nombre, apell y edad****
>
> typedef struct sData****
>
> {****
>
> char Nombre[30];****
>
> char Apellido[30];****
>
> char Edad[10];****
>
> }tiposData;****
>
> ** **
>
> //Creo el nodo que va a contener el tipo de dato     tiposData dato;****
>
> ** **
>
> typedef struct Nodo****
>
> {****
>
>     tiposData dato;****
>
>     struct Nodo *anterior,*siguiente;****
>
> } tipoNodo;****
>
> ** **
>
> ** **
>
> typedef tipoNodo *pNodo;****
>
> typedef tipoNodo *Lista;****
>
> ** **
>
> ** **
>
> void Insertar(Lista *lista, tiposData v) {
>

En general, deberías pasar la dirección de una estructura en lugar de la
estructura en sí. Esto es,

void Insertar( Lista *lista, const tiposData *pDato );

Obviamente, habría que tratarlo como un puntero en el código de la
implementación de esta función.

****
>
>    pNodo nuevo, actual;****
>
> ** **
>
>    /* Crear un nodo nuevo */****
>
>    nuevo = (pNodo)malloc(sizeof(tipoNodo));****
>
>    nuevo->dato = v;****
>
> ** **
>
>    /* Colocamos actual en la primera posición de la lista */****
>
>    actual = *lista;****
>
>    if(actual) while(actual->anterior) actual = actual->anterior;****
>
> **
>

Este bucle no es necesario. El doble puntero 'lista' apunta al primer nodo
de la lista. No veo que necesites "retroceder". De hecho, esto puede ser
problemático, si la lista apuntada realmente representa una sublista. Esto
implica que 'Insertar()' no será general, al ser restrictiva al manejo de
listas; es decir, no permite sublistas.

**
>
>    /* Si la lista está vacía o el primer miembro es mayor que el nuevo */*
> ***
>
>    if(!actual || strncmp(actual->dato.Nombre,v.Nombre) > 0) {
>

Esto es incorrecto. La función 'strncmp()' requiere tres parámetros; el
tercero indica la cantidad máxima de caracteres en la cadena del primer
parámetro. En este caso, debería ser:

strncmp( actual->dato.Nombre, v.Nombre, 30 )

****
>
>       /* Añadimos la lista a continuación del nuevo nodo */****
>
>       nuevo->siguiente = actual;****
>
>       nuevo->anterior = NULL;****
>
>       if(actual) actual->anterior = nuevo;****
>
>       if(!*lista) *lista = nuevo;****
>
>    }****
>
>    else {****
>
>       /* Avanzamos hasta el último elemento o hasta que el siguiente tenga
> ****
>
>          un valor mayor que v */****
>
>       while(actual->siguiente &&
> strncmp(actual->siguiente->dato.Nombre,v.Nombre)< 0)
>

Esto es un error. Hemos llegado a esta sentencia debido a que la condición
de 'if' es falsa. Como la condición se basa en una expresión compleja, no
sabes cuál subcondición falló: 'actual' o 'strncmp()'. Si la subcondición
que contiene 'strncmp()' es falsa, entonces no tenemos graves problemas,
pero si 'actual' es nulo, entonces sí tendremos muchos problemas al
comenzar este bucle 'while', porque accedes a un miembro de una estructura
que no existe.


Además, tienes el mismo error anterior. Escribe:

strncmp( actual->siguiente->dato.Nombre, v.Nombre, 30 )


> ****
>
>       {****
>
>           actual = actual->siguiente;****
>
>       /* Insertamos el nuevo nodo después del nodo anterior */****
>
>       nuevo->siguiente = actual->siguiente;****
>
>       actual->siguiente = nuevo;****
>
>       nuevo->anterior = actual;****
>
>       if(nuevo->siguiente) nuevo->siguiente->anterior = nuevo;****
>
>       }
>

Podrías hacer todo esto, pero sinceramente no lo recomiendo. No obtienes
ninguna ventaja al asignar repetidamente punteros a 'nuevo'. El bucle en sí
sirve para buscar el nodo adecuado; de hecho, tu propio comentario así lo
dice.

Una solución alternativa es simplemente buscar el nodo en el bucle. Una vez
terminada la búsqueda - y fuera del bucle - es cuando preparamos 'nuevo'
con los vínculos encontrados.

****
>
>    }****
>
> }****
>
> **
>

Aconsejo tratar el caso de la lista nula por separado y luego puedes mirar
el caso general, teniendo en cuenta que la lista no está vacía.

**
>
> ** **
>
> ** **
>
> //a la funcion mostrar solo se le pasa la lista****
>
> void mostrar(Lista *l)
>

No es necesario pasar el puntero de la lista. También ten presente que
'mostrar()' no tiene intención alguna de modificar el contenido de la lista.

Sugiero hacer esto:

void mostrar( const Lista l )

****
>
> {****
>
>     Lista aux;****
>
>     aux=l;
>

Esto es un error, ya que los tipos no concuerdan. Debería haber sido:

aux = *l;

Ahora bien, si aplicas la sugerencia que hice antes sobre el tipo del
parámetro, entonces puedes dejar esta sentencia como está. Eso sí, tendrías
que cambiar el tipo de 'aux' para que concuerde con el del parámetro 'l'.
Esto es,

const Lista aux;

****
>
>
>
[CORTE]

int main(void)****
>
> {****
>
>     printf("Programa para crear una lista de datos\n ");****
>
>     Lista l;****
>
>     l=(Lista)malloc(sizeof(tipoNodo));****
>
>     l->anterior=l->siguiente=NULL;
>

No aconsejo crear memoria ahora mismo, porque en realidad la lista está
vacía. Por lo tanto, sólo debería ser:

Lista l = NULL;

Obviamente, no asignaríamos valores a los campos del nodo, porque no existe
tal nodo ahora mismo.


> ****
>
>     int opcion=1;****
>
>     tiposData dato;****
>
>     do****
>
>     {****
>
>         printf("Si desea ingresar un dato presione 1 de lo contrario salga
> con cualquier letra\n");****
>
>         scanf("%d",&opcion);****
>
>         if(opcion==1)****
>
>         {****
>
>            dato = ingresarDatos();****
>
>            Insertar(&l,dato);****
>
>            mostrar(&l);****
>
>         }****
>
>     }while(opcion==1);****
>
> ** **
>
> ** **
>
>     return 0;****
>
> }****
>

Tienes un serio problema al final, porque no has desadjudicado la memoria
que adjudicaste con 'malloc()'. Debes invocar 'free()' para cada nodo que
creaste. Esto implica que debes recorrer la lista invocando 'free()' para
cada nodo apuntado.


Espero que esto te aclare las dudas.

Steven
------------ próxima parte ------------
Se ha borrado un adjunto en formato HTML...
URL: <http://listas.conclase.net/pipermail/cconclase_listas.conclase.net/attachments/20130215/973b49eb/attachment.html>