2

In my program I have a function called freeFx();

this function is fed two arrays and a count of records to free.

I am getting an invalid pointer error when this function is called. I don't quite understand where this error is coming from, any help would be awesome!

here is the code:

/* ---- LIBRARIES ---- */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* ---- PROTOTYPES ---- */
int readFx(char** charArr, int* intArr);

/* int sortFx(char** arr, int arg2);
int printFx(char** arr, int arg2); */

int freeFx(char** charArr, int* intArr, int cnt);
char* getToken(char arr1[], int loc);
void makeRoom(char*** t, int** z, int size);

/* ---- MAIN ---- */
int main(void)
{
    char** charPntrArr;
    int* intPntrArr;
    char* fileText;
    int iniArrSize = 10;
    int recCnt = 0;
    int i = 0;

    /* array to store addresses of arrays forming the rows */

    charPntrArr = malloc(iniArrSize * sizeof(char*));
    intPntrArr = malloc(iniArrSize * sizeof(int));

    recCnt = readFx(charPntrArr, intPntrArr);
    printf("%d\n", recCnt);

    /*sortFx(pntrArr, recCnt);
     printFx(pntrArr, recCnt); */
    freeFx(charPntrArr, intPntrArr, recCnt);

    return;
}

/* ---- FUNCTIONS ---- */

int readFx(char** charArr, int* intArr)
{
    /*
    input: csv file of string arrays
    output: count of records received
    purpose: read file, store values in array and populate pointer array
    */

    char buffer[350];
    char temp[350];
    char temp2[350];
    char*** reallocTemp;
    char* token;
    int counter;
    int subLoc = 4;
    int enrLoc = 9;
    int arrSize = 10;

    /* Clear headers */
    fgets(buffer, sizeof(buffer), stdin);

    counter = 0;

    /* While file stream is not null */
    while (fgets(buffer, sizeof(buffer), stdin) != NULL)
    {

        /* Populate array within array if pntr arr has room */
        if (counter < arrSize)
        {

            /* buffer copy*/
            strcpy(temp, buffer);
            strcpy(temp2, buffer);

            /* create array for token values*/
            charArr[counter] = malloc(10 * sizeof(char));

            /* Get first token */
            token = getToken(temp, subLoc);

            strcpy(charArr[counter], token);

            /* Get second token */
            token = getToken(temp2, enrLoc);

            intArr[counter] = atoi(token);

            counter++;
        }
        else
        {
            /* Reallocate memory due to necessary expansion */
            makeRoom(&charArr, &intArr, arrSize);

            /* Realloc was successful */
            if (temp != NULL)
            {
                arrSize = arrSize * 2;

                /* Print Reallocation info */
                printf("reallocating to %d\n", arrSize);

                /* Populate values for current buffer now that you have realloc'd */

                /* buffer copy*/
                strcpy(temp, buffer);
                strcpy(temp2, buffer);

                /* create array for token values */
                charArr[counter] = malloc(10 * sizeof(char));

                /* Get first token */
                token = getToken(temp, subLoc);

                strcpy(charArr[counter], token);

                /* Get second token */
                token = getToken(temp2, enrLoc);

                intArr[counter] = atoi(token);

                counter++;
            }
            else
            {
                printf("unable to reallocate\n");
                exit(1);
            }
        }



    }
        return counter;
}

    char* getToken(char arr1[], int loc)
    {

        /*
        input: string array & location of desired string
        output: string of token at position
        purpose: grab string (char*) of certain position in given array
        */

        int loopCnt;
        char* del = ",\n";

        /* Grab first token */
        char* token = strtok(arr1, del);

        /* Loop through array to grab value at given location */
        for (loopCnt = 1; loopCnt < loc; loopCnt++)
        {
            token = strtok(NULL, del);
        }

        return token;
    }

    int freeFx(char** charArr, int* intArr, int cnt)
    {
        int i;

        printf("INSIDE FREE FX\n");
        for (i = 0; i < cnt; i++)
        { 
            printf("%d\n", i);
            free(charArr[i]);
        }
        printf("FREED ARRAYS WITHIN ARRAY\n");

        free(charArr);
        printf("CHAR ARR FREE\n");

        free(intArr);
        printf("INT ARR FREE\n");

        return 0;
    }

    void makeRoom(char*** t, int** z, int size)
    {   
        *t = (char**)realloc(*t, size * 2 * sizeof(char*));

        *z = (int*)realloc(*z, size * 2 * sizeof(int*));

    }

HERE IS A SAMPLE FROM THE TEXT FILE:

Term Code,Session Code,Campus Code,Subject,Catalog Nbr,Section,Class Nbr,Class Component,Enrollment Total,Enrollment Cap,Enrollment Availability,Waitlist Total,Waitlist Cap,Instructor Name,Instructor Email,Building Code,Room Nbr,Start Time,End Time,M,T,W,Th,F,Sa,Su,Class Start Date,Class End Date
2152,1,MAIN,SOCW,6390,6,22913,IND  - Independent Study,0,1,1,0,0,,,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,MUSI,4460,3,21831,PRI  - Private Lesson,0,20,20,0,0,Michael J Drake,mjdrake@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,MAE,4301,1,27674,LEC  - Lecture,0,5,5,0,3,,,NH,109,7:00 PM,8:20 PM,Y,N,Y,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,EE,2403,101,25557,LAB  - Laboratory,11,24,13,0,0,Jonathan W Bredow,jbredow@uta.edu,NH,148A,5:30 PM,8:20 PM,Y,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,SOCW,6451,68,26055,PRA  - Practicum,1,1,0,0,0,Laura S Frank,laura.frank@mavs.uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,ARCH,3331,1,20182,LEC  - Lecture,47,61,14,0,0,Edward R Nelson,nelsone@uta.edu,ARCH,401,5:30 PM,6:50 PM,Y,N,Y,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,BIOL,6291,7,26391,IND  - Independent Study,0,5,5,0,0,Matthew Fujita,mkfujita@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,BE,6194,12,30366,IND  - Independent Study,6,6,0,0,0,Young-Tae Kim,ykim@uta.edu,,,,,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,BIOL,5698,21,27536,THE  - Thesis Research,0,5,5,0,0,Laura D Mydlarz,mydlarz@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,EDAD,6399,7,20089,DTN  - Dissertation,2,10,8,0,0,Daniel B Saunders,saunders@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,BE,3344,14,26082,LEC  - Lecture,6,10,4,0,0,Baohong Yuan,baohong@uta.edu,ERB,131,11:00 AM,12:20 PM,N,Y,N,Y,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,EDAD,6390,11,26017,LEC  - Lecture,0,10,10,0,0,Yi Zhang,lyzhang@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,BIOL,3454,2,20468,LAB  - Laboratory,31,30,-1,0,0,Nicholas A Long,nicholas.long@mavs.uta.edu,LS,133,1:00 PM,4:50 PM,Y,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,CHEM,1451,1,22411,LEC  - Lecture,118,140,22,0,0,Seiichiro Tanizaki,tanizaki@uta.edu,SH,121,9:00 AM,9:50 AM,Y,N,Y,N,Y,N,N,1/20/2015,5/8/2015
2152,1,MAIN,ME,6297,39,30394,IND  - Independent Study,1,5,4,0,0,Ashfaq Adnan,aadnan@uta.edu,,,,,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,MUSI,1243,2,21463,PRI  - Private Lesson,1,20,19,0,0,Young-Hyun Cho,yhcho@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,MUSI,4242,2,21728,PRI  - Private Lesson,0,20,20,0,0,Young-Hyun Cho,yhcho@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
2152,1,MAIN,EVSE,6399,44,25290,DTN  - Dissertation,1,5,4,0,0,Merlynd K Nestell,nestell@uta.edu,,,12:00 AM,12:00 AM,N,N,N,N,N,N,N,1/20/2015,5/8/2015
Dpry12
  • 105
  • 8
  • Is there some particular reason you're allocating an `int` sequence using sizing based on `int*` ? And potentially related, [stop casting `malloc` in C](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – WhozCraig Mar 11 '16 at 07:29
  • @WhozCraig, nope, none whatsoever! Thanks for this, corrected it! Any idea on the invalid pointer error? The error is coming from the first call of free(charArr[i]); – Dpry12 Mar 11 '16 at 07:31
  • I don't suppose you could throw up the first 30 or so lines of your input file into your question in a separate text list? And honestly, this is screaming for the business-end of a debugger. – WhozCraig Mar 11 '16 at 07:44
  • Have you tried checking if (NULL == charPntrArr) after malloc ? – jonystorm Mar 11 '16 at 07:46
  • @WhozCraig added a sample of the file above – Dpry12 Mar 11 '16 at 07:49
  • @jonystorm, not quite sure I'm following you on this one...are you talking about the initial malloc? or the 2nd malloc? or both? – Dpry12 Mar 11 '16 at 07:55
  • Um. Do you realize that data is NOT comma delimited? As such the first "token" for each line will be *the entire line*. The delimiter list `",\n"` means just keep scanning until a comma or a newline or terminator are found. Is that data *tab* delimited ? – WhozCraig Mar 11 '16 at 07:58
  • There's no harm on checking after each malloc – jonystorm Mar 11 '16 at 07:58
  • lol sorry @WhozCraig, I just copy and pasted straight from the excel version of the csv...let me try again...its good to go now! – Dpry12 Mar 11 '16 at 08:00
  • Thanks for the updates. – WhozCraig Mar 11 '16 at 08:05
  • no, thank you @WhozCraig! I really appreciate the help! – Dpry12 Mar 11 '16 at 08:06

2 Answers2

3

From what I see, besides the brittleness of the code in general (there is MUCH I would change), the biggest problem is by-value arrays you're passing in to your reader function.

You initially size them here in main():

charPntrArr = malloc(iniArrSize * sizeof(char*));
intPntrArr = malloc(iniArrSize * sizeof(int)); // note: fixed this. also fix in your resize fn

Then their values (the addresses held by each of those pointers) are passed into your reader here:

recCnt = readFx(charPntrArr, intPntrArr);

At various times the reader can (and does) resize those buffers, including relocating data if needed. There is no guarantee the base address stays the same. Thus when readFX returns, the pointers still hold the original values, but the resize efforts have long-since made those locations no longer defined as accessible.

A quick way to address this is to do the following:

  • Change your readFX function to take its pointer parameters by address (pointers to pointers).
  • Rename the input parameters in the actual function (it will make sense when you see the code below).
  • Declare local vars that have the same original names, and are initialized with the dereferenced initial values of your params.
  • Save the locals back to the dereferenced parameters when the function finishes.

Something like this:

int readFx(char*** ppCharArr, int** ppIntArr)
{
    char **charArr = *ppCharArr;
    int *intArr = *ppIntArr;

    /*
     input: csv file of string arrays
     output: count of records received
     purpose: read file, store values in array and populate pointer array
     */

    char buffer[350];
    char temp[350];
    char* token;
    int counter;
    int subLoc = 4;
    int enrLoc = 9;
    int arrSize = 10;


    counter = 0;

    /* Clear headers */
    fgets(buffer, sizeof(buffer), stdin);

    /* While file stream is not null */
    while (fgets(buffer, sizeof(buffer), stdin) != NULL)
    {

        /* Populate array within array if pntr arr has room */
        if (counter >= arrSize)
        {
            /* Reallocate memory due to necessary expansion */
            arrSize = makeRoom(&charArr, &intArr, arrSize);

            /* Realloc was successful */
            if (charArr == NULL || intArr == NULL)
            {
                printf("unable to reallocate\n");
                exit(1);
            }
        }

        /* buffer copy*/
        strcpy(temp, buffer);

        /* Get first token */
        token = getToken(buffer, subLoc);
        if (token != NULL)
            charArr[subLoc] = strdup(token);

        /* Get second token */
        token = getToken(temp, enrLoc);
        intArr[counter] = atoi(token);

        counter++;

    }

    *ppCharArr = charArr;
    *ppIntArr = intArr;
    return counter;
}

Invoked from main() like this:

charPntrArr = malloc(iniArrSize * sizeof *charPntrArr);
intPntrArr = malloc(iniArrSize * sizeof *intPntrArr);
recCnt = readFx(&charPntrArr, &intPntrArr);

The freeFX call can stay as it is. That's about the quickest way I can see for you to resolve this specific issue. Note: I did some hacking up on this code, so some things will not work with a simple cut/paste back to your code base (I have makeRoom returning the new size, for example), but you can hopefully still see what the root problem was.

Hope it helps.


Update

A dumbed down version with integrated allocation in the read-array and bubble-sorting of the content. I hope the OP finds it useful. This is considerably cleaner, imho, than the original version.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* ---- PROTOTYPES ---- */
int readFx(char*** ppCharArr, int** ppIntArr);
int freeFx(char** charArr, int* intArr, int cnt);
void sortFx(char** const charArr, int* const intArr, int cnt);

/* ---- MAIN ---- */
int main(void)
{
    char** charPntrArr = NULL;
    int* intPntrArr = NULL;
    int recCnt = 0;

    /* array to store addresses of arrays forming the rows */

    /* read file and get record count */
    recCnt = readFx(&charPntrArr, &intPntrArr);
    sortFx(charPntrArr, intPntrArr, recCnt);
    freeFx(charPntrArr, intPntrArr, recCnt);
}

/* ---- FUNCTIONS ---- */

int readFx(char*** ppCharArr, int** ppIntArr)
{
    const int subLoc = 4;
    const int enrLoc = 9;

    char **charArr = *ppCharArr;
    int *intArr = *ppIntArr;

    char line[350];
    char* token = NULL;
    int arrSize = 0;
    int counter = 0;
    int i=0;

    /* Clear headers */
    fgets(line, sizeof(line), stdin);
    while (fgets(line, sizeof(line), stdin) != NULL)
    {
        // check for reallocation prior to insertion
        if (counter == arrSize)
        {
            // need to expand
            int newSize = (arrSize ? (2*arrSize) : 1);
            void *tmp = realloc(charArr, newSize * sizeof *charArr);
            if (tmp == NULL)
            {
                fprintf(stderr, "Failed to expand charArr to %d elements", newSize);
                exit(1);
            }
            charArr = tmp;

            // expand intArr likewise
            tmp = realloc(intArr, newSize * sizeof(*intArr));
            if (tmp == NULL)
            {
                fprintf(stderr, "Failed to expand intArr to %d elements", newSize);
                exit(1);
            }
            intArr = tmp;
            arrSize = newSize;

            printf("Resized arrays to %d slots\n", newSize);
        }

        // get tokens
        for (token = strtok(line, ",\n"), i=1; token && (i<subLoc); ++i)
            token = strtok(NULL, ",\n");
        if (token)
        {
            charArr[counter] = strdup(token);

            // next token
            for (; token && i<enrLoc; ++i)
                token = strtok(NULL, ",\n");
            if (token)
            {
                intArr[counter] = atoi(token);
                printf("%s %d\n", charArr[counter], intArr[counter]);
                ++counter;
            }
            else
            {
                free(charArr[counter]);
            }
        }

    }

    *ppCharArr = charArr;
    *ppIntArr = intArr;

    return counter;
}

int freeFx(char** charArr, int* intArr, int cnt)
{
    while (cnt--)
        free(charArr[cnt]);
    free(charArr);
    free(intArr);
    return 0;
}


void sortFx(char** const charArr, int* const intArr, int cnt)
{
    int swapped = 1, i, j=cnt;

    // simple bubblesort algorithm. note there is no string copying cone here
    //  we compare strings, and if swapping is needed, swap *pointers* in the
    //  charArr pointer array (and intArr side by side, but that is unrelated)
    while (swapped && j--)
    {
        swapped = 0; // reset swap detection

        for (i = 0; i < j; ++i)
        {
            int cmp = strcmp(charArr[i], charArr[i+1]);
            if ( cmp > 0)
            {
                char *strTmp = charArr[i];
                int intTemp = intArr[i];

                // do the swaps
                charArr[i] = charArr[i+1];
                charArr[i+1] = strTmp;
                intArr[i] = intArr[i+1];
                intArr[i+1] = intTemp;

                // something swapped so set flag
                swapped = 1;
            }
            else if (cmp == 0 && intArr[i] > intArr[i+1])
            {
                int intTemp = intArr[i];
                intArr[i] = intArr[i+1];
                intArr[i+1] = intTemp;

                // something swapped so set flag
                swapped = 1;
            }
        }
    }

    printf("\nSORTED RESULTS\n");
    for (i=0; i<cnt; ++i)
        printf("%s %d\n", charArr[i], intArr[i]);
}

Output

The following output is from the pasted sample data.

Resized arrays to 1 slots
SOCW 0
Resized arrays to 2 slots
MUSI 0
Resized arrays to 4 slots
MAE 0
EE 11
Resized arrays to 8 slots
SOCW 1
ARCH 47
BIOL 0
BE 6
Resized arrays to 16 slots
BIOL 0
EDAD 2
BE 6
EDAD 0
BIOL 31
CHEM 118
ME 1
MUSI 1
Resized arrays to 32 slots
MUSI 0
EVSE 1

SORTED RESULTS
ARCH 47
BE 6
BE 6
BIOL 0
BIOL 0
BIOL 31
CHEM 118
EDAD 0
EDAD 2
EE 11
EVSE 1
MAE 0
ME 1
MUSI 0
MUSI 0
MUSI 1
SOCW 0
SOCW 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • This helps a lot, thank you. Another thing I guess I didn't mention, we aren't allowed to use file pointers, we have to use stdin. Is there any way I can still utilize this without that? – Dpry12 Mar 11 '16 at 08:23
  • @Dpry12 I stuffed your sample data in a local text file. Sorry, it was the easiest way for me to debug it. That should not have anything to do with the problem I pointed out. As I said in the answer, this code will NOT just cut/paste into your code base and "work" The problem I point out is, none-the-less, completely valid. I took liberty to put reading back on `stdin` regardless. – WhozCraig Mar 11 '16 at 08:24
  • Ok, so here is a link to my updated coding, taking your advice. [LINK](https://drive.google.com/file/d/0B3JyOtVloC3-SlhONUZXanRkRms/view?usp=sharing)...now I am getting a "warning: no newline at end of file" error – Dpry12 Mar 11 '16 at 08:34
  • You seem to have a pretty high warning level (good for you). C translation units (source file as far as you're concerned) need to end with a newline to be standard-compliant. Go to the end of your source file and smack the enter/return key. save it, and compile again (if I understand that warning correctly). It should still compile and run, however. – WhozCraig Mar 11 '16 at 08:38
  • Great. For added homework, think about how you can do this without an initial pair of allocations in `main()` (i.e. just passing in two `NULL` pointers by address to your reader. It is actually more complicated doing it like you are now. Best of luck. – WhozCraig Mar 11 '16 at 08:40
  • Fingers crossed you're still around. I have another issue now...I am trying to sort the two arrays now alphabetically based upon the char* array...if you could help out with why I'm getting a segmentation error, I would love you long time. Here is another [LINK](https://drive.google.com/file/d/0B3JyOtVloC3-aWI1VnFNWVcwSE0/view?usp=sharing)...I tried implementing what we talked about above, but having trouble – Dpry12 Mar 11 '16 at 09:24
  • You seriously need to run this under a debugger and inspect the values of pointers, indexes, etc, to determine where the wheels are falling off. A seg-fault will trip a debugger where the fault happened, and in all likelihood nearly immediately assist in rooting out your issue. – WhozCraig Mar 11 '16 at 20:11
-1

check the return value of readFx.

Might just be easier to post the whole function. Maybe counter isn't starting at 0

Tibrogargan
  • 4,508
  • 3
  • 19
  • 38
  • Just updated it to show all of the code! Thanks for your help in advance...also return value is 999, which is correct since we are given a file with 1000 lines and I ditch the header row at the beginning – Dpry12 Mar 11 '16 at 07:36
  • jonystorm is on the right track I think. I was wondering why you were always allocating 10 chars for your text. It looks like your text tokens can be bigger than that. Might want to allocate strlen(token) + 1 instead. – Tibrogargan Mar 11 '16 at 07:49
  • the 10 chars is due to the fact that I know the values from column 4 will never exceed 10. Sorry if that was confusing... – Dpry12 Mar 11 '16 at 07:51