Valgrind: Invalid readsize of 1. strcpy

Question

I am recieving a seg fault from valgrind i am not exactly sure what the error means or how to actually fix it. I am hoping for some clarification on the error and a potential fix. This is the error: I

==8063== Command: ./main stock.dat coins.dat ==8063==

==8063== Invalid read of size 1

==8063== at 0x4C2E1C7: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==8063== by 0x401046: setupNode (in /home/joshua/Documents/Assignment 2/main)

==8063== by 0x400E78: main (in /home/joshua/Documents/Assignment 2/main)

==8063== Address 0x0 is not stack'd, malloc'd or (recently) free'd

==8063==

==8063==

==8063== Process terminating with default action of signal 11 (SIGSEGV)

==8063== Access not within mapped region at address 0x0

==8063== at 0x4C2E1C7: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==8063== by 0x401046: setupNode (in /home/joshua/Documents/Assignment 2/main)

==8063== by 0x400E78: main (in /home/joshua/Documents/Assignment 2/main)

==8063== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

    #include "vm_menu.h"
    #include "vm_type.h"
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <malloc.h>
    #define NUMARGS 3
    void addNodeBottom(struct stock_item *val, struct stock_item *head);
    struct stock_item* setupNode(char* line);
    char* openFile(char fileName[]);
    int main(int argc, char * argv[])
    {
        struct vm vm;
        struct menu_item menu_items[NUM_MENU_ITEMS];


        /* The UNUSED() function is designed to prevent warnings while your
         * code is only partially complete. Delete these 4 function calls once
         * you are using the data structures declared above in your own code */
        UNUSED(argc);
        UNUSED(argv);
        UNUSED(vm);
        UNUSED(menu_items);
        //creates a the first element for the stock_items type in the data structure.
        struct stock_item *root;
        struct stock_item *item;
        item = (struct stock_item *) malloc( sizeof(struct stock_item) );


        /* Now rootNode points to a node struct */
        root = (struct stock_item *) malloc( sizeof(struct stock_item) );

        /* List of things to do in this function: */
        /* check command line arguments */
        if(argc!=3){
            printf("insuffcient arguments \n");
            return EXIT_SUCCESS;
        }
        /*open stock file*/
        char* fileName = argv[1];
        FILE *file;
        file = fopen(fileName,"r+");

        /*read file*/
        long lSize;
        char *buffer;
        fseek( file , 0L , SEEK_END);
        lSize = ftell( file);
        rewind( file );
        /* allocate memory for entire content */
        buffer = calloc( 1, lSize+1 );
        if( !buffer ) fclose(file),fputs("memory alloc fails",stderr),exit(1);
        /* copy the file into the buffer */
        if( 1!=fread( buffer , lSize, 1 , file) )
            fclose(file),free(buffer),fputs("entire read fails",stderr),exit(1);
        fclose(file);
        /*parse the file using | as the delmeter case*/
        //declare all variables used to store the string in.
        int counter = 0;
        const char newLine[2] = "\n";
        char *saveptr;
        char *line;
        int size;



        //parse the id and set it first.
        size = sizeof(strtok(buffer,newLine)); // allocated memory to store the id in.
        line = malloc(size); //allocates the memory

        int active = 0;

        line = strtok_r(buffer,newLine, &saveptr); // copies the first line from the text file into a char array. .


        while (!active){
            if (line == NULL) {
                printf("\n%s", "Reached end of file while parsing.");
                return(0);
            }

            printf("%s",line);
            item = setupNode(line);
            line = strtok_r(NULL,newLine, &saveptr);

            addNodeBottom(item, root);
            // free(item);
        }


        return 0;
    }
    char* openFile(char fileName[]){
        FILE *file;

        file = fopen(fileName,"r+");

        /*read file*/
        long lSize;
        char *buffer;



        fseek( file , 0L , SEEK_END);
        lSize = ftell( file);
        rewind( file );

        /* allocate memory for entire content */
        buffer = calloc( 1, lSize+1 );
        if( !buffer ) fclose(file),fputs("memory alloc fails",stderr),exit(1);

        /* copy the file into the buffer */
        if( 1!=fread( buffer , lSize, 1 , file) )
            fclose(file),free(buffer),fputs("entire read fails",stderr),exit(1);
        //printf("%s", buffer);
        //      printf("%s",content);

        free(buffer);

        fclose(file);
        return buffer;

    }
    struct stock_item* setupNode(char* line){
        struct stock_item *roots = {NULL};
        roots = (struct stock_item *) malloc( sizeof(struct stock_item)+1 );

        char *ptr;
        char *prices;
        const char del[2] = "|";
        const char delm[2] = ".";
        strcpy(roots->id, strtok_r(line,del,&ptr)); // returns the ID and stores in in the root node.
        strcpy(roots->name, strtok_r(NULL,del,&ptr)); // returns the description and stores it in the root node.
        strcpy(roots->description, strtok_r(NULL,del,&ptr)); // returns the description and stores it in the root node.
        prices = strtok_r(NULL,del,&ptr); // returns a string of the price for vm_item.

        int dol = atoi(strtok(prices,delm));
        int cent = atoi(strtok(NULL,delm));
        roots->price.dollars = dol;
        roots->price.cents = cent;
        int quantity = atoi(strtok_r(NULL,del,&ptr)); // returns how many items are in stock.
        roots->on_hand = quantity;
        return roots;
    }


    void addNodeBottom(struct stock_item *val, struct stock_item *head){

        //create new node
        struct vm_node *newNode = (struct vm_node*)malloc(sizeof(struct vm_node));
        if(newNode == NULL){
            printf("%s", "Unable to allocate memory for new node\n");
            exit(-1);
        }

        newNode->data= val;
        //  printf("%s",newNode->data->id);
        newNode->next = NULL;  // Change 1

        //check for first insertion
        if(head->next == NULL){
            head->next = newNode->data;

            printf("\nadded at beginning\n");
        }
        else
        {

            //else loop through the list and find the last
            //node, insert next to it
            struct vm_node *current = head;
            while (TRUE) { // Change 2
                if(current->next == NULL)
                {
                    current->next = newNode;
                    printf("\nadded later\n");
                    break; // Change 3
                }
                current = current->next;
            };

        }


    }

The output for the code when it runs is:

I0001|Coke|75 ml Can of coke|3.50|50

added at beginning

I0002|Pepsi|375 ml Can of pepsi|3.00|20

added later

I0003|Lemon Cheesecake|A delicious, 1/8 size slice of cheesecake|4.00|10

added later

I0004|Mars Bar|A delicious 50 g Mars Bar chilled just the way you like it.|3.00|20

added later

I0005|Lemon Tart|A delicious lemon butter tart with a pastry based|3.75|12

added later

Reached end of file while parsing.

this output is generated using eclipse, when i run it in terminal it just responds with a segfault with no display.

    #ifndef VM_TYPE
    #define VM_TYPE

    #define IDLEN 5
    #define NAMELEN 40
    #define DESCLEN 255
    #define NUMDENOMS 8
    #define UNUSED(var) (void)var
    #define COIN_COUNT 20
    #define DEFAULT_ONHAND 20

    /* Type definition for our boolean type */
    typedef enum truefalse
    {
        FALSE, TRUE
    } BOOLEAN;

    /* Each price will have a dollars and a cents component */
    struct price
    {
        unsigned dollars,cents;
    };

    /* The different denominations of coins available */
    enum denomination
    {
        FIVE_CENTS, TEN_CENTS, TWENTY_CENTS, FIFTY_CENTS, ONE_DOLLAR, 
        TWO_DOLLARS, FIVE_DOLLARS, TEN_DOLLARS
    };

    /* Each coin in the coins array will have a denomination (20 cents, 
     * 50 cents, etc) and a count - how many of that coin do we have on hand
     */
    struct coin
    {
        enum denomination denom;
        unsigned count;
    };

    /* The data structure that holds the data for each item of stock
     */
    struct stock_item
    {
        char id[IDLEN+1];
        char name[NAMELEN+1];
        char description[DESCLEN+1];
        struct price price;
        unsigned on_hand;
        struct stock_item *next;
    };

    /* The data structure that holds a pointer to the stock_item data and a
     * pointer to the next node in the list
     */
    struct vm_node
    {
        struct stock_item * data;
        struct vm_node * next;
    };

    /* The head of the list - has a pointer to the rest of the list and a 
     * stores the length of the list 
     */
    struct vm_list
    {
        struct vm_node * head;
        unsigned length;
    };

    /* This is the head of our overall data structure. We have a pointer to 
     * the vending machine list as well as an array of coins. 
     */
    struct vm
    {
        struct vm_list * item_list;
        struct coin coins[NUMDENOMS];
        char * foodfile;
        char * coinsfile;
    };

    #endif

These are all the type def's

and this is the file structure that is being read.

I0001|Coke|75 ml Can of coke|3.50|50

I0002|Pepsi|375 ml Can of pepsi|3.00|20

I0003|Lemon Cheesecake|A delicious, 1/8 size slice of cheesecake|4.00|10

I0004|Mars Bar|A delicious 50 g Mars Bar chilled just the way you like it.|3.00|20

I0005|Lemon Tart|A delicious lemon butter tart with a pastry based|3.75|12

Answer 1

The error Address 0x0 is not stack'd, malloc'd or (recently) free'd means that strcpy's source is a NULL pointer. Obviously, strtok_r is returning NULL rather than what you're expecting.

You should rewrite your code in such a manner that if your input is not in the correct format, you detect the NULL rather than crashing. (There are other problems with your code).

Answer 2

size = sizeof(strtok(buffer,newLine));
line = malloc(size); //allocates the memory

This is wrong: sizeof yields the sizeof the returned pointer (strtopk returns a char *) which is basically 4 or 8 bytes.