Help! Segmentation fault error with simple C ++ list

  

When it is the case that the node is added before the first term it works fine, but when I must add it after the first one I continually skip the error .

It seems like a perfect case to use a debugger step by step, it would be a good way to practice:)

The problem is localized, so let's debug the function insertar in second call for a N greater than the content in actual->edad :

void insertar(int N){
    nodo *nuevo=new nodo(N);nodo *aux=primero; actual=primero;
    bool k=0;
    int i=0;
    if (listaVacia()){
        primero=nuevo;
    }else{
        while(actual || k==0){
            if(actual->edad>N){
                nuevo->siguiente=actual;
                if (i==0) primero=nuevo;
                k=1;
            }else if (i==0){
                actual=actual->siguiente;
            }else{
                actual=actual->siguiente;
                aux=aux->siguiente;
            }
                i++;
        }
        if (k==0){
            actual=aux;
            actual->siguiente=nuevo;
        } 
    }
}

Assuming:

• Address (invented) of primero : 0x00000001 .
• primero->edad = 10 .
• primero->siguiente = NULL .

We call insertar(666) :

In step 19, you have skipped (with the arrow operator -> ) from memory address NULL , this causes your error since you can not skip from NULL or de-reference NULL .

Your insertar function in addition to malfunctioning, it's too complex, take a look at this answer which resolves what you need.

Well, in part Rene Garnica is right, your code is a bit complex for what it does , I've re-implemented a much simpler version of your code, without the need for circular references and those details.

Compared to your code, what you are looking for with the class lista is to access the elements of this, which is practically unnecessary, to have a Nodo you only need its value and the next one in the stack, for what we implement this class Nodo :

class Nodo {
    public:
        Nodo *Siguiente;
        int Valor;
        Nodo(int);
        Nodo(Nodo*);
};

And its respective implementation:

// Implementación clase Nodo:
Nodo::Nodo(int a) {
    Valor = a;    // Se crea un nuevo nodo con un valor.
    Siguiente = NULL;
}
Nodo::Nodo(Nodo *n) {
    this->Valor = n->Valor;        // Se utiliza como constructor copia
    if (n->Siguiente != NULL)     
        this->Siguiente = n->Siguiente;
    else 
        this->Siguiente = NULL;
} 
// Fin implementación.  

As well as class Lista :

class Lista {
    public:
        Lista();
        void Insertar(int);
        void Mostrar();
        Nodo *Nodos;
};

And its implementation:

//Implementacion clase Lista:
Lista::Lista() {
    Nodos = NULL;
}
void Lista::Insertar(int a) {
    if (Nodos == NULL) Nodos = new Nodo(a);
    else {
        Nodos->Siguiente = new Nodo(Nodos);
        Nodos->Valor = a;
    }
}

void Lista::Mostrar() {
    Nodo *Actual = new Nodo(Nodos);

    while (Actual != NULL) {
        cout << "Actual: " << Actual->Valor << '\n';
        if (Actual->Siguiente == NULL) break;
        else
            Actual = new Nodo(Actual->Siguiente);
    }
}

In this main() :

#include <iostream>

using namespace std;

int main() {
    Lista *T = new Lista(); int a = 0, count = 0, max = 0;
    cout << "Inserte la cantidad maxima de elementos a introducir: "; cin >> max;
    while (count < max) {
        cout << "Escriba el elemento "<< count + 1 << " de " << max << ": "; cin >> a;
        T->Insertar(a); // Insertamos el elemento en la lista.
        ++count;
    }
    T->Mostrar();

    cin.get();
    return 0;
}

The problem of Segmentation Fault is because you are trying to access a memory address that is not being used or that is no longer used, so we need to foresee this if we do not want problems to the time to implement our solution.

If you look at the constructor of Nodo , there are two constructors, the reason for this is that the memory addresses are not shared when using the pointer to the object, but a new instance is created and then reuse those that are already.

I have tried this code and it offers me the following result:

Inserte la cantidad maxima de elementos a introducir: 10
Escriba el elemento 1 de 10: 1
Escriba el elemento 2 de 10: 2
Escriba el elemento 3 de 10: 3
Escriba el elemento 4 de 10: 4
Escriba el elemento 5 de 10: 523
Escriba el elemento 6 de 10: 23423
Escriba el elemento 7 de 10: 235
Escriba el elemento 8 de 10: 23
Escriba el elemento 9 de 10: 535
Escriba el elemento 10 de 10: 1234
Actual: 1234
Actual: 535
Actual: 23
Actual: 235
Actual: 23423
Actual: 523
Actual: 4
Actual: 3
Actual: 2
Actual: 1

Hello personally I think you have a bad design of the list as you commented it is not necessary to create circular references if you need to access the attributes of the node class, leave them public or implement their getter chords but do not do any more.

Based on your code, I have re-implemented the list class so that it works personally. It is more complicated to find the error than to simply redo the code adding that your implementation is somewhat confusing.

Finally, you have to implement the destructor of the class because at no time do you release the created nodes.

#include <iostream>

using namespace std;

class nodo
{
public:
    int edad;
    nodo *siguiente;

    nodo(int N)
    {
        edad=N;
        siguiente=NULL;
    };
};

class lista
{
private:
    nodo *cabeza;
public:

    lista (){
        cabeza=NULL;
    }

    //comprueba si la lista esta vacia
    bool listaVacia(){
        return (cabeza==NULL);
    }

    void insertar(int N){

        nodo *nuevo=new nodo(N);

        if(cabeza==NULL){
            cabeza=nuevo;
        }
        else{

            nodo *temp=cabeza;

            while(temp->siguiente)
            {
                temp=temp->siguiente;
            }
            temp->siguiente=nuevo;
        }
    }

    //Muestra los elementos de la lista
    void mostrar(){
        nodo *temp=cabeza;

        while(temp){
            cout<<temp->edad<<"--->";
            temp=temp->siguiente;
        }
        cout<<"NULL"<<endl;
    }


    //Destructor de la lista libera la memoria de los nodos
    ~lista(){

        while(cabeza){

            nodo *temp=cabeza;
            cabeza=cabeza->siguiente;
            delete temp;
        }
    }
};

int main()
{
    lista newlista;
    int A;

    for(int i=0; i<10; i++)
    {
        cout<<"Ingrese primera edad"<<endl;
        cin>>A;
        newlista.insertar(A);
        newlista.mostrar();
    }

    return 0;
}