Segmentation fault C++

Question

#include <stdio.h>
#include <iostream>

using namespace std;

struct nodeDouble{
  int data;
  nodeDouble* next;
  nodeDouble* prev;
};

nodeDouble* head;
nodeDouble* tail;

void addNode(int number){
  if(head == NULL){
    head = (nodeDouble*)malloc(sizeof(struct nodeDouble));
    head->data = number;
    head->next = NULL;
    head->prev = NULL;
    tail = head;
  }else{
    tail->next = (nodeDouble*)malloc(sizeof(struct nodeDouble));
    tail->next->data = number;
    tail->next->prev = tail;
    head->next->next = NULL;
    tail = tail->next;
  }
}

void printList(nodeDouble* headList){

  while(headList != NULL){
    cout << headList->data << "\n";
    headList = headList->next;
  }
  headList = NULL;
}

int main(){
  for (int i = 0; i < 15; i++) {
    addNode(i);

  }
  printList(head);
  return 0;
}

Answer 1

Fíjate en esta línea aparentemente inofensiva (se encuentra en addNode):

head->next->next = NULL;

Esta línea está rompiendo tu lista enlazada. En el momento en el que añadas tres elementos, esta línea cortará la lista. Es decir, tu lista quedará así:

head                    tail
node -> node -> null    node -> null

Cuando tu esperas que quede así:

head            tail
node -> node -> node -> null

Y claro, si tu iteras desde head hasta tail sin comprobar punteros acabas encontrándote con ese tercer nodo que es null y ahí empiezan los problemas.

Dado que estás en C++ te sugiero, al igual que hacen en la otra respuesta, que dejes de usar malloc y pases a usar new. Junto con este cambio, define un constructor apropiado y el problema se solucionará solo:

struct nodeDouble{
  int data;
  nodeDouble* next;
  nodeDouble* prev;

  nodeDouble(int value)
    : data(value),
      next(nullptr),
      prev(nullptr)
  { }
};

nodeDouble* head = nullptr;
nodeDouble* tail = nullptr;

void addNode(int number){
  nodeDouble * node = new nodeDouble(number);

  if(head == nullptr){
    head = node;
  }else{
    tail->next = node;
    node->prev = tail;
  }

  tail = node;
}

Lo que sucede ahora es que new invoca, implícitamente, al constructor que hemos declarado en la clase. Este constructor inicializa los punteros next y prev a nullptr (sustituto natural de NULL en el estándar C++11). Además también inicializa data a partir de un valor que le facilitamos.

Posteriormente solo actualizaremos aquellos punteros que necesitemos y listo, la lista estará correctamente enlazada.

También prodríamos crear un constructor un poco más completo:

struct nodeDouble{
  int data;
  nodeDouble* next;
  nodeDouble* prev;

  nodeDouble(int value, nodeDouble* previous)
    : data(value),
      next(nullptr),
      prev(previous)
  {
    if( nullptr != previous )
      previous->next = this;
  }
};

nodeDouble* head = nullptr;
nodeDouble* tail = nullptr;

void addNode(int number){
  nodeDouble * node = new nodeDouble(number,tail);

  if(head == nullptr)
    head = node;

  tail = node;
}

Ahora el constructor recibe también un puntero al nodo anterior. Si este puntero es nulo no hace nada, pero si no lo es se añade al final de la lista, por lo que solo faltaría actualizar tail.

Answer 2

Los Nodos no son Listas.

Este es un error recurrente en StackOverflow en Español que genera mucha confusión.

En el código que has facilitado estás pasando variables de tipo nodeDouble con nombre de lista y eso es tan erróneo como decir que un escalón es una escalera, sinceramente ¿Te parecen lo mismo?:

La nomenclatura es importante.

Además de la incorrecta nomenclatura que confunde nodos con listas, tu nodo se llama nodeDouble pero los datos contenidos son int ¿Seguro que estás haciendo lo que quieres?

Tu pregunta es sobre C++.

La cabecera <stdio.h> es de c no de c++. Esta cabecera dispone de una versión adaptada a C++ que tiene el prefijo c y carece de extensión. Si realmente necesitas usar las cabeceras de C (que nunca será el caso) debes usar los equivalentes de C++ <cstdio> . Lee este hilo para saber por qué.

La función de alojamiento de memoria std::malloc pertenece a C, no a C++. En C++ se usa new que lanza una excepción (std::bad_alloc ) si falla al alojar memoria, tiene tipado fuerte, el tamaño del objeto para el que se aloja memoria es calculado por el compilador a través del tipo, separa el alojado de objetos del de formaciones y se puede sobrecargar. Lee este hilo para saber más.

El lenguaje C++ es multiparadigma, así que a priori no estás limitado a un paradigma concreto; pero uno de los puntos fuertes del lenguaje es su soporte a la programación orientada a objetos así que te aconsejo que realmente crees un objeto lista en lugar de confiar en funciones sueltas.

Propuesta.

Teniendo en cuenta todo lo anterior, tu código podría tener este aspecto:

class Lista
{
    struct node{
      int data = 0;
      node* next = nullptr;
      node* prev = nullptr;
    };

    node* head = nullptr;
    node* tail = nullptr;

    friend std::ostream &operator<<(std::ostream &, const Lista &);

public:
    void add(int number);
};

La clase Lista dispone de una clase interna en la zona privada que es el node; esta clase privada es inaccesible desde fuera favoreciendo el encapsulamiento: la propia clase gestiona sus nodos, desde fuera de la lista no hay motivos para trabajar con nodos, he renombrado addNode a add por ese motivo.

Con este código, la implementación de add podría parecerse a:

void Lista::add(int number)
{
    if (tail)
    {
        tail = tail->next = new node{number, nullptr, tail};
    }
    else
    {
        head = tail = new node{number};
    }
}

El código anterior aprovecha que node es un agregado para poder asignar valores al construir; en la propia construcción se enlaza con los nodos correspondientes.

También he substituido la función printList por una función amiga que escribe en el flujo de salida, pudiendo usar la lista así:

Lista l;
l.add(3);
l.add(2);
l.add(1);

std::cout << l;

Puedes ver el código funcionando en Wandbox.

Answer 3

Prueba a inicializar head y tail a null:

nodeDouble* head = NULL;
nodeDouble* tail = NULL;

Aunque en C++ deberías evitar usar punteros (o, si tienes que usarlos, intenta usar smart pointers, como std::unique_ptr). Si vas a usar punteros, intenta usar nullptr (en vez de NULL).

Por otro lado, intenta evitar malloc (en la gran mayoría de casos, usar malloc es undefined behavior en C++), usa new:

head = new nodeDouble();