0

I have Node and Queue structs in the file queue.h

#ifndef QUEUE_H
#define QUEUE_H

#include<stdio.h>
#include<stdbool.h>

typedef struct rcb
{
    int seq_no;     
    int file_desc;   
    FILE *fp;      
    long sent;      
    long sizeOfFile;
    char *request_http_method;
    char *request_file;
    char *request_http_version;
    int level;         
    bool is_new_created;
    int remaining;
}* RCB;    

/*
 * Node of the queue.
 */
typedef struct node {
    RCB info;       //Data of each node.
    struct node *ptr;//pointer to the next node.
} * Node;

/*
 * Queue for the requests.
 */
typedef struct req_queue {
    Node front;     //front node.
    Node rear;      //rear node.
    int size;       //size of the queue.
} * Queue;


/**
 * Create the queue.
 */
void create(Queue queue);


/**
 * For inserting an item to the queue in sorted order of file size.
 * @param data
 */
void enque_SJF(Queue queue, RCB data);

#endif

queue.c

 #include "queue.h"

Node temp, front1;
int count = 0;    

/**
 * For creating the queue.
 */
void create(Queue queue) {
    queue->front = queue->rear = NULL;
    queue->size=0;
}

/**
 * Enqueing in the order of increasing file size.
 * @param data
 */
void enque_SJF(Queue que, RCB data) {
    bool found = false;
    Node temp = que->front;
    while (!found) {
        if (que->front == NULL) { //if the queue is empty.
            que->front = malloc(sizeof (struct node));
            que->front->ptr = NULL;
            que->front->info = data;
            que->rear = que->front;
            break;
        } else {
            if (temp->ptr == NULL) { 
                Node newnode = (struct node *) malloc(1 * sizeof (struct node));
                newnode->info = data;
                if (temp->info->sizeOfFile >= data->sizeOfFile) { 
                    newnode->ptr = temp;
                    que->front = newnode;
                    break;
                } else { //else enqueue at the rear.
                    temp->ptr = newnode;
                }
            } else {
                if (temp == que->front && temp->info->sizeOfFile >= data->sizeOfFile) { 
                    Node newnode = (struct node *) malloc(1 * sizeof (struct node));
                    newnode->info = data;
                    newnode->ptr = temp;
                    que->front = newnode;
                    break;
                }

                if (temp->ptr->info->sizeOfFile >= data->sizeOfFile) { 
                    Node newnode = (struct node *) malloc(1 * sizeof (struct node));
                    newnode->info = data;
                    newnode->ptr = temp->ptr;
                    temp->ptr = newnode;
                    break;
                } else
                    temp = temp->ptr;
            }
        }
    }
    que->size++;
}

I am trying to enqueue a new node to a queue in the function enque_SJF in queue.c file. The enqueue function is called in sws.c by the function serve_client function. Here is the sws.c There are some more functions in those files but they are not related with my problem so i didnt wrote all functions to be more simple;

#include "Queue.h"
#include "network.h"
#include "schedulers.h"
#include "shared.h"

char scheduler[4];
pthread_t tid[2];
int port;
Queue req_queue;
bool flag[2];
int turn;
int sequence_number;


void  *serve_client()
{
    static char *buffer;                              /* request buffer */
    int fd;    

    req_queue = (struct req_queue *) malloc( sizeof (struct req_queue));
    create(req_queue);

    if (port != 0)
    {
        network_init( port );                             /* init network module */
        fprintf(stderr, "Connection port %d\n", port);
        for( ;; )
            /* main loop */
        {
            network_wait();
            if( !buffer )                                     /* 1st time, alloc buffer */
            {
                buffer = malloc( MAX_HTTP_SIZE );
                if( !buffer )                                   /* error check */
                {
                    perror( "Error while allocating memory" );
                    abort();
                }
            }

            for( fd = network_open(); fd >= 0; fd = network_open() )   /* get clients */
            {
                memset( buffer, 0, MAX_HTTP_SIZE );
                if( read( fd, buffer, MAX_HTTP_SIZE ) <= 0 )      /* read req from client */
                {
                    perror( "Error while reading request" );
                    abort();
                }
                printf("file path %s\n",buffer);
                //Initializing memory for the job.
                RCB request = (RCB) malloc(1 * sizeof (struct rcb));

                //breaking the request in appropriate format.
                request-> request_http_method = strtok(buffer, " "); //request method.
                request->request_file   = strtok(NULL, " /"); //request file
                request->request_http_version = strtok(NULL, "\n"); //HTTP version
                request->file_desc = fd;
                request->level = 1; // for multilevel scheduler.
                request->seq_no = sequence_number;
                sequence_number++; //increment global counter.


                    enque_SJF(req_queue, request); //Enqueue for Shortest Job First.


            }
        }
    }
    return 0;
}


bool isValidRequest(RCB request)
{

    // the request is parsed and checked the validity
}


void *SJF( )
{
   // function implemented
}




int main(int argc, char **argv )
{
    //default port, if no port is supplied.
    /* check for and process parameters
    */
    if( ( argc < 3 ) || ( sscanf( argv[1], "%d", &port ) < 1 ) || ( sscanf( argv[2], "%s", scheduler ) < 1 ) )
    {
        printf("port %d\n",port);
        printf("port %s\n",scheduler);
        printf( "usage: sms <port> <scheduler>\n" );
        return 0;
    }
    sequence_number = 1; //counter for number of requests.
    if (argc == 3)
    {
        port = atoi(argv[1]);
    }
     printf("port %d\n",port);
    pthread_create(&(tid[0]), NULL, serve_client, NULL);

    if(strcmp(scheduler,"SJF") ==0)
    {
        pthread_create(&(tid[1]), NULL, SJF, NULL);
    }
    else if(strcmp(scheduler,"RR")==0)
    {
        pthread_create(&(tid[1]), NULL, Round_Robin, NULL);
    }
    else if(strcmp(scheduler,"MLFB")==0)
    {
        pthread_create(&(tid[1]), NULL, MultilevelQueueWithFeedback, NULL);
    }
    else
    {
        printf("Scheduler Algorithm is not defined. Please enter one of them; SJF, RR, MLFB");
        return 0;
    }

    pthread_join(tid[0], NULL);
    pthread_join(tid[1], NULL);


    return 0;


}

While adding a new node to the queueu i am getting a segmentation fault error at the following line;

que->front->ptr = NULL;

While debugging, i see that after memory allocation the address for que->front is still 0x0. Is there any suggestion why it does not allocate memory?

Emel Uras
  • 394
  • 2
  • 13
  • 1
    [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Jun 21 '16 at 05:33
  • and don't typedef pointers, please. – Sourav Ghosh Jun 21 '16 at 05:34
  • check if `malloc()` is success. – Sourav Ghosh Jun 21 '16 at 05:34
  • Enough with the cast discussion really. Nothing in that post is more than an opinion – Ishay Peled Jun 21 '16 at 05:42
  • How did you declare `que` and did you assing memory to it? Please provide a complete code example. – Support Ukraine Jun 21 '16 at 05:46
  • i edited question. I removed casting for allocation and i an still getting error. – Emel Uras Jun 21 '16 at 05:52
  • You are still not providing a complete example. You added `req_queue` but the code uses `que`. How does `req_queue`turn in to `que`. Is the code inside a function? Then show how the function is declared and called. And what is the `break`doing? The `break` doesn't make sense with the posted code. – Support Ukraine Jun 21 '16 at 05:55
  • I edited the question. – Emel Uras Jun 21 '16 at 14:22
  • 1
    In your `serve_client()` function, when you fill out `request` you're setting a lot of members to pointers into `buffer` (via `strtok()`), rather than making copies of the relevant strings. The next time you erase or fill `buffer`, the data those pointers point to won't be valid anymore since you will have overwritten it. – Dmitri Jun 21 '16 at 15:30
  • Also, when you add a node to the rear, you're not updating `que->rear` (though I don't see anywhere that you actually check it in the code shown). – Dmitri Jun 21 '16 at 15:52

1 Answers1

1

That is why you should always perform success check for the called function(s).

In case, if malloc() is failure, it will return a NULL pointer which gets stored in que->front. After that, without a NULL check, if you try to access

 que->front->ptr

you'll be de-referencing a NULL pointer (i.e, accessing invalid memory) which invokes undefined behavior.

Always do a NULL check on the return value of malloc().

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261