[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 531
  • Last Modified:

Problem with queue in C

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
Tom3333
Asked:
Tom3333
1 Solution
 
Minh Võ CôngCommented:
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
 
tampnicCommented:
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
 
Tom3333Author Commented:
What is the function new QUEUE(); how to define this function???
0
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
tampnicCommented:
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
 
sarabandeCommented:
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
 
ambienceCommented:
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
 
ambienceCommented:
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

Important Lessons on Recovering from Petya

In their most recent webinar, Skyport Systems explores ways to isolate and protect critical databases to keep the core of your company safe from harm.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now