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== 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

Also, you're sure that no strtok (or strtok_r) call returns NULL? — Some programmer dude
Finally, don't use the comma operator to separate statements, it makes the code hard to read and understand. Instead use proper blocks, i.e. put the statements inside curly-bracers {}. Also instead of reading the whole file into memory and parsing lines manually, why not simply use fgets to get lines from the file directly? — Some programmer dude
@JoachimPileborg i had not considered using fgets. Ill look up how to use it. Thanks for the suggestion. — Joshua Theeuf

o11c o11c · Accepted Answer · 2014-10-06T08:00:18

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).

Valgrind: Invalid readsize of 1. strcpy

2 Answers