Solved

Problem with queue in C

Posted on 2012-04-12
7
461 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
Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

 
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 33

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

Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

IntroductionThis article is the second in a three part article series on the Visual Studio 2008 Debugger.  It provides tips in setting and using breakpoints. If not familiar with this debugger, you can find a basic introduction in the EE article loc…
This is a short and sweet, but (hopefully) to the point article. There seems to be some fundamental misunderstanding about the function prototype for the "main" function in C and C++, more specifically what type this function should return. I see so…
The goal of this video is to provide viewers with basic examples to understand opening and reading files in the C programming language.
The goal of the video will be to teach the user the difference and consequence of passing data by value vs passing data by reference in C++. An example of passing data by value as well as an example of passing data by reference will be be given. Bot…

929 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

14 Experts available now in Live!

Get 1:1 Help Now