0

I am writing a program that makes an undirected graph and performs BFS on that graph. This graph uses an adjacency list representation, built on-top of a List struct that I had previously built. Anyways, I'm trying to load in the graph from a file then perform BFS on it, but if I perform BFS on a graph (loaded from a file) with over 30 vertices, I get a segfault during a malloc call. If I, however, don't load the graph from file and instead make it by hand (using loops and manually adding vertices/edges), I can have thousands of vertices and 10k+ edges and no segfaults occur at all. This obviously makes me think that something that I'm doing while loading in the graph data is corrupting the heap, but the odd thing is that the malloc call that fails isn't in the function that reads from the file, and the function does correctly translate the input file into a graph data structure, so I am lost. Below is the function that reads in from the input file.

The following is my output on GDB :

Program received signal SIGSEGV, Segmentation fault.
_int_malloc (av=0x7ffff7dd3760 <main_arena>, bytes=24) at malloc.c:3766
3766    malloc.c: No such file or directory.

I would like to stress that there has to be something inside of this function that is causing my heap to corrupt, even though the corruption doesn't show until a bit later.

Graph readFile(char * fn) {

  FILE * fp = fopen(fn, "r");
  char line[80];

  if(fp == NULL) {
    fprintf(stderr, "Error : Invalid Filename Argument");
    exit(1);
  }

  fgets(line, 80, fp);
  int order = 0;
  sscanf(line, "%d", &order);
  if(order <= 0) {
    fprintf(stderr, "Error parsing input file, order < 0");
    exit(1);
  }
  printf("%d\n", order);
  Graph new_graph = newGraph(order);

  while(fgets(line, 80, fp) != NULL)
  {
    int origin = -1;
    int terminus = -1; 
    sscanf(line, "%d %d", &origin, &terminus);
    printf("%d %d\n", origin, terminus);
    if(origin > 0 && terminus > 0 && origin <= order && terminus <= order) {
      addEdge(new_graph, origin , terminus);
    }
    else {
      break;
    }
  }
   fclose(fp);
  //printGraph(stdout, new_graph);
   return new_graph;
}

If I call this function to load a graph, then perform BFS, the following malloc call fails in a function inside the List class

void append(List L, int data) {
    if(L == NULL) {
        fprintf(stderr, "Error : L null in append\n");
        exit(1);
    }

    printf("Before\n");
    Node * new_node = malloc(sizeof(Node)); <-- SEGFAULTS
    printf("After\n");
    if(new_node == NULL) {
      fprintf(stderr, "Error : Malloc Fail in Append");
    }
    new_node->data = data;

    // printf("Midway\n");
    if(L->num_nodes == 0) {
        L->front_node = new_node;
        L->back_node = new_node;
        L->num_nodes++;
        return;
    }
    new_node->prev = L->back_node;
    L->back_node->next = new_node;
    L->back_node = new_node;
    L->num_nodes++;
}
jhallard
  • 23
  • 8
  • 6
    Most common case is a heap corruption. You have probably `free`ed memory that is corrupted. Further `malloc` calls expect that the internal heap structures are correct but they aren't. But it can be sufficient to write beyond the allocated storage to corrupt the heap internals. – harper Feb 26 '15 at 07:47
  • okay that makes sense, do you happen to see any place in the readFile function that would be doing something like that? Like I said, the data structures themselves are solidly tested on much larger and more complex data-sets and everything went fine, it's only when reading into the graph using that function that things later fail. – jhallard Feb 26 '15 at 07:54
  • 1
    You might like to go for a memory checker like Valgrind (http://valgrind.org). – alk Feb 26 '15 at 07:54
  • 1
    @harper is right wrt heap corruption. Another error source is, I think, writing outside the allocated object (because bookkeeping information is stored between the "officially" allocated blocks which gets destroyed and sends malloc into the nether). Therefore check carefully your array boundaries. For debugging this kind of error with gcc you may find http://www.gnu.org/software/libc/manual/html_node/Heap-Consistency-Checking.html interesting, or this SO question: http://stackoverflow.com/questions/1010106/how-to-debug-heap-corruption-errors (with helpers in Win and *nix). – Peter - Reinstate Monica Feb 26 '15 at 07:54
  • It looks like your vertices are numbered starting at 1. If you have an array of vertices in your graph, did you allocate an extra space in you array for the last vertex? – JS1 Feb 26 '15 at 08:06
  • I do a conversion internally to convert from 1-n to 0-(n-1) and reconvert when displaying. I thought that was a problem as well but I double checked and I'm not failing in that area. – jhallard Feb 26 '15 at 08:23

1 Answers1

1

So I fixed it somehow.. it turns out not to have to do with my readFile function, for some reason that just brought the error out, even though I was only reading in a couple vertices compared to the thousands I was testing on before. To solve the problem, I simply changed every single

Node * new_node = malloc(sizeof(Node))

call to the following

Node * new_node = newNode();

and made the following function

Node * newNode(void) {
    Node * new_node = malloc(sizeof(struct NodeObj));
    if(new_node == NULL) {
        fprintf(stderr, "Error : Malloc returned null node");
        exit(1);
    }
    new_node->prev = NULL;
    new_node->next = NULL;
    new_node->data = -1;
    return new_node;
}

Hope that helps someone in the future, I still don't know why my heap was corrupted but it seems to be working better now.

jhallard
  • 23
  • 8
  • 1
    In fact you added `new_node->next = NULL;` which most probably fixes the issue. – alk Feb 26 '15 at 08:30
  • that makes sense, TBH I'm a C++ programmer by heart so for some reason I had assumed that the values inside Node were being initialized by a default constructor. – jhallard Feb 26 '15 at 09:10
  • 1
    Hm. It would help to see the declarations of Node and nodeObj. If the relation between them is simply a definition like `Node nodeObj;` then it shouldn't matter whether you use the type or the object. @alk: How does nulling the pointer help? I cannot see a test for null in the code. (Excepth that any iteration would possibly test for null as an end condition, sure.) – Peter - Reinstate Monica Feb 26 '15 at 09:33
  • 1
    @PeterSchneider: We do not see all the code. And if the `next` member would not be used it wouldn't be there ... I suspect. – alk Feb 26 '15 at 09:35