Solved

Problem with queue in C

Posted on 2012-04-12
7
451 Views
Last Modified: 2012-05-02
i tried to created a queue in c but when i run the code i saw that a  segmentation fault is appear.
What is going wrong?

The code is :

typedef struct node
      {     char name[MAX];
            int id;
            int size_node;
            struct node *next;
      }NODE;

typedef struct
      {
            NODE *head;
            NODE *tail;
      }QUEUE;

void InitialiseQueue(NODE q);
void add(NODE *q,char name[],int id);
void display(NODE *);

int main(void)
{
NODE *q;
InitialiseQueue(*q);
add(q,"A1",11);
return 0;
}


void InitialiseQueue(NODE q)
            {
            NODE *head=NULL;
            NODE *tail=NULL;
            }
void add(NODE *q,char name[],int Identity,int time,char Target[])
      {
       NODE *temp;
        temp = q;
              if(q==NULL)
                    {
                      q=(NODE *)malloc(sizeof(NODE));
                      temp = q;
                    }
            else
                    {
                      while((temp=temp->next)!=NULL);
                      temp->next = (NODE *)malloc(sizeof(NODE));
                      temp=temp->next;
                    }
                     memcpy(temp->name,name,MAX);
                    temp->id=id;
                    temp->size_node=temp->size_node+1;
                    temp->next  = NULL;
      }    
      

void display(NODE *pt)
{
  while(pt!=NULL)
  {
    printf("element is");
    printf("%s\n",pt->name);
    pt=pt->next;
  }
}
0
Comment
Question by:Tom3333
7 Comments
 
LVL 15

Expert Comment

by:Minh Võ Công
ID: 37840937
You change:
int main(void)
{
QUEUE *q = new QUEUE();
q->head = NULL;
q->tail= NULL;
add(q,"A1",11);
/* free memory before return
*/
return 0;
}


void add(QUEUE *q,char name[],int Identity,int time,char Target[])
 {
       NODE *temp= (NODE *)malloc(sizeof(NODE));
       memcpy(temp->name,name,MAX);
       temp->id=id;
       temp->size_node=temp->size_node+1;
       temp->next  = NULL;
       if (q->head == NULL)
       {
           q->head = tmp;
           q->tail = tmp;
       }else {
            q->tail->next = tmp;
            q->tail = tmp;
      }
                 
}
0
 
LVL 7

Expert Comment

by:tampnic
ID: 37842182
If this is not for an academic exercise you should really be using the STL containers (see http://www.cplusplus.com/reference/stl/queue/) to implement a queue. The STL code has been across many many programmers eyes  and utilised in thousands of applications - you couldn't write more reliable and verifiable code by yourself!

Cheers,
   Chris
0
 

Author Comment

by:Tom3333
ID: 37842251
What is the function new QUEUE(); how to define this function???
0
Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

 
LVL 7

Expert Comment

by:tampnic
ID: 37842337
We seem to have an issue with the zones for this question - its been put in the C++ zone so I have given a C++ answer (use the STL). the new function is also C++, in ANSI C you would say
 QUEUE *q = malloc(sizeof(QUEUE));

Open in new window

 or use an operating-system-specific memory allocation routine if suitable

Cheers,
   Chris
0
 
LVL 32

Expert Comment

by:sarabande
ID: 37843833
in C you would pass the QUEUE by pointer to InitializeQueue:


void InitialiseQueue(QUEUE * pq)
{
      pq->head=NULL;
      pq->tail=NULL;
}

Open in new window


in main you would create a QUEUE by

int main()
{
      QUEUE q;
      InitialiseQueue(&q);
      ...
}

and then pass to InitialiseQueue by pointer using the & .

the working add function for C was posted by minhvc though i would use strncpy and not memcpy.

size_t len = strlen(name);
if (len >=  sizeof(temp->name))
   len = sizeof(temp->name) -1;
strncpy(temp->name, name, len);
temp->name[len] = '\0';

Open in new window


Sara
0
 
LVL 22

Expert Comment

by:ambience
ID: 37844448
Few points:

1) You may want to put size_node in QUEUE rather than NODE (I judging that given your code)

typedef struct node
      {     char name[MAX];
            int id;
            struct node *next;
      }NODE;

typedef struct
      {
            NODE *head;
            NODE *tail;
            int size_node;
      }QUEUE;

2) Add wouldnt do what its supposed to. Also when it comes to queues, generally (as convention) you use enqueue and dequeue (but add/remove are OK too). I would suggest that you pass the QUEUE to add

3) add will segfault for sure - see bolded lines

void add(NODE *q,char name[],int Identity,int time,char Target[])
      {
       NODE *temp;
        temp = q;
              if(q==NULL)
                    {
                      q=(NODE *)malloc(sizeof(NODE)); // memory leak as soon as the method exits
                      temp = q;
                    }
            else
                    {
                      while((temp=temp->next)!=NULL);
                      temp->next = (NODE *)malloc(sizeof(NODE)); // temp will always be NULL here
                      temp=temp->next; // whats that for? Will blow up code following this
                    }
                    memcpy(temp->name,name,MAX); // if name is NULL it will blowup
                    temp->id=id;
                    temp->size_node=temp->size_node+1; // this should be in QUEUE
                    temp->next  = NULL;
      }
0
 
LVL 22

Accepted Solution

by:
ambience earned 50 total points
ID: 37844486
Also,

while((temp=temp->next)!=NULL);

beats the purpose of tail.

I recommend you to fatten the following boilerplate

NODE* makeNewNode(const char* name. int identity, int time, const char* target)
{
      // Add validation here, check for null name and target and return if null
      // allocate new node, set data and return
}

void enqueue(QUEUE* queue, const char* name. int identity, int time, const char* target)
{
// Add validation here, check for null queue
NODE* newNode =makeNode(name, identity, time, target);
// Add error check here for NULL node

if(queue->head == NULL)
{
      // update head and tail and set both to same new node
}
else
{
     // update tail - head remains unchanged
}

// update queue node_size
}

NODE* dequeue(QUEUE* queue)
{
// Add validation here, check for null queue
NODE* temp = queue->head;
// Advance head, if head is NULL then set tail NULL too
return temp;
}
0

Featured Post

IT, Stop Being Called Into Every Meeting

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

Join & Write a Comment

Introduction This article is a continuation of the C/C++ Visual Studio Express debugger series. Part 1 provided a quick start guide in using the debugger. Part 2 focused on additional topics in breakpoints. As your assignments become a little more …
Summary: This tutorial covers some basics of pointer, pointer arithmetic and function pointer. What is a pointer: A pointer is a variable which holds an address. This address might be address of another variable/address of devices/address of fu…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn how to clear a vector as well as how to detect empty vectors in C++.

747 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

12 Experts available now in Live!

Get 1:1 Help Now