segmentation fault during the use of an array of pointer to structures

Question

I have to save in an array the address of some data. Every data is a structure of type "dagNode". To do my work I visit a list, and I count the number of data that I want to record its address so I allocate the right space in the memory, and finally I re-visit the list and save the address of some data.

    struct dagNode *buildBST(struct dagNode *rootList){
    struct dagNode *head, **xTest;
    head = rootList;
    int numXtest=0;

    rootList = nextNode(TYPE_XTEST, rootList);

    while ( !IS_TERMINATED( rootList ) ){   // first visit
        numXtest++;
        rootList = nextNode(TYPE_XTEST, rootList); }

    xTest = (struct dagNode **) malloc( sizeof(struct dagNode ) * numXtest);
    int i=0; rootList = nextNode(TYPE_XTEST, head);

    for(i=0; i<numXtest; i++){      // second visit, saving the address of some datas
        rootList = nextNode(TYPE_XTEST, rootList);
        xTest[i] = rootList; i++;
    >>> printf("t=%d,val=%d\t", xTest[i]->nodeType, xTest[i]->val); } // segmentation fault

    return head;
    }

EDIT:

    struct dagNode *nextNode(int typeOfNextNode, struct dagNode *node){
        if (IS_TERMINATED(node)){   return node;    }
        node = node->next;
        if (typeOfNextNode == TYPE_EDGE_OR_GAP){
            while (!IS_TERMINATED(node) && !IS_AN_EDGE(node) && !IS_A_GAP(node)){
                node = node->next;  }
               }else
        {
            while (!IS_TERMINATED(node) && (node->nodeType != typeOfNextNode)){
                node = node->next;}
        }
        return node;    }

You can isolate the problem by running your code in the debugger. — Oliver Charlesworth, Feb 14 '12 at 14:35
That is the most inscrutable code layout I've seen in a long time. Please, for everyone's sanity's sake, adopt a more orthodox format. At the least, the `if` and the `else` should align vertically, and the `}` should be at the start of the line, not hidden at the end. — Jonathan Leffler, Feb 14 '12 at 14:52
Never typecast the result of malloc in C. Read [this](http://stackoverflow.com/questions/1565496/specifically-whats-dangerous-about-casting-the-result-of-malloc). — Lundin, Feb 14 '12 at 15:23

UmNyobe · Answer 1 · 2012-02-14T15:11:00.057

2

nextNode() seems to be the obvious culprit.

Edit: Note that you are also incrementing i twice per iterations. This will certainly crash.

edited Feb 14 '12 at 15:11

answered Feb 14 '12 at 14:36

UmNyobe

22,539
9
61
90

1

This is the most probable cause of the trouble. In the loop, there is a hidden increment `xTest[i] = rootList; i++;` just before the print. So, you skip past the pointer you've just set up before you print the data. Since the `for` loop control also increments `i`, you (the OP) should probably just delete the `i++;` inside the loop. The memory issue identified by @Blagovest is also something to deal with, but if `sizeof(struct dagNode) >= sizeof(struct dagNode *)` (which is inevitable since the nodes contain a pointer), the defect only wastes memory rather than causing crashes. – Jonathan Leffler Feb 14 '12 at 15:00

score 1 · Accepted Answer · answered Feb 14 '12 at 14:38

There is a mismatch between the type of the xTest pointer and the size you allocate in malloc. If xTest is of type struct dagNode **, the proper allocation should be:

xTest = malloc(sizeof(struct dagNode *) * numXtest);

Probably you want to allocate numXtest pointers to a struct, not numXtest structs.

score 0 · Answer 3 · answered Feb 14 '12 at 16:03

I'm sorry but this post does not answer the question. I merely want to give you an example of an alternative coding style. Pick up what you like from it.

I haven't changed the meaning of the code, except for the removal of the most obvious bugs already pointed out in other answers/comments.

#include <stdio.h>
#include <stdlib.h>

typedef struct
{
  ...
} dagNode;

dagNode* buildBST (dagNode* rootList)
{
  dagNode*  head;
  dagNode** xTest;
  int numXtest=0;
  int i=0;

  head = rootList;
  rootList = nextNode(TYPE_XTEST, rootList);

  while (!IS_TERMINATED(rootList))               // first visit
  {
    numXtest++;
    rootList = nextNode(TYPE_XTEST, rootList);
  }

  xTest = malloc(sizeof(dagNode) * numXtest);
  rootList = nextNode(TYPE_XTEST, head);

  for(i=0; i<numXtest; i++)                      // second visit, saving the address of some datas
  {      
    rootList = nextNode(TYPE_XTEST, rootList);
    xTest[i] = rootList;

    printf("t=%d,val=%d\t", 
           xTest[i]->nodeType,
           xTest[i]->val);
  }

  return head;
}


dagNode *nextNode (int typeOfNextNode, dagNode *node)
{
  if (IS_TERMINATED(node))
  {
    return node;
  }

  node = node->next;

  if (typeOfNextNode == TYPE_EDGE_OR_GAP)
  {
    while ( !IS_TERMINATED(node) &&
            !IS_AN_EDGE(node) &&
            !IS_A_GAP(node) )
    {
      node = node->next;
    }
  }
  else
  {
    while ( !IS_TERMINATED(node) &&
            (node->nodeType != typeOfNextNode) )
    {
      node = node->next;
    }
  }

  return node;
}

Thanks for your post, i'm following your (better) style in the future. — optimusfrenk, Feb 25 '12 at 11:52

segmentation fault during the use of an array of pointer to structures

3 Answers3