Hey, I have here a piece of code that works in parts but does not work together. If I test all three parts at once, I get a segmentation fault immediately before the program is supposed to test for a (null) value in the string array. However, if I comment out the first part of the program, testing the first set of functions, the other two test fine. The same applies to commenting out the tests of the second set of functions.

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

int ht, wd;    /* height and width for array in part 1 */ 

 
    /*functions for part 1*/

int * create2DArray(int height, int width)
{
    int * array = malloc(height * width * sizeof(int));

    return array;    
}


void set2DElement(int * array, int loc_x, int loc_y, int value)
{
    array[loc_x + wd*loc_y] = value;
}


int get2DElement(int * array, int loc_x, int loc_y)
{
    return array[loc_x + wd*loc_y];
}


void free2DArray(int * array)
{
    free(array);
}


    /* functions for part 2 */
char ** createStringArray(int num)
{
    char **stringPtr = malloc(num * sizeof(char *));
    return stringPtr;
}


void setStringArray(char ** stringPtr, int position, char * setstring)
{
    stringPtr[position] = malloc(strlen(setstring) * sizeof(char));
    strcpy (stringPtr[position], setstring);
}


char * getStringArray(char ** stringPtr, int position)
{
    if(stringPtr[position] == NULL){
        return " ";
    }
    return stringPtr[position];
}


void freeStringArray(char ** stringPtr, int num)
{
    int x = 0;

    for (x = 0; x < num; x++)
    {
        free(stringPtr[x]);
    }

    free(stringPtr);
}



    /* functions for part 3 */
int ** createArray(int width, int height)
{    
    int x = 0;
    int ** matrix = malloc(height * sizeof(int *));
    
    for (x = 0; x < height; x++)
    {
        matrix[x] = malloc (width * sizeof(int));
    }
    
    return matrix;
}


void freeArray(int ** matrix, int height)
{
    int x = 0;

    for (x = 0; x < height; x++)
    {
        free(matrix[x]);
    }
    
    free(matrix);
}


int main(int argc, char *argv[]) {
    int *array;
    int width, height;
    int value;
    char **stringPtr;
    int **matrix;



    printf("Testing Part 1\n");
    width = 5;
    height = 6;

    array = create2DArray(height, width);

    printf("Store value 7 at [3,4].\n");
    set2DElement(array, 3, 4, 7);

    value = get2DElement(array, 3, 4);
    printf("Retrieve value %d from [3,4]\n\n", value);

    printf("Store value 200 at [0,4].\n");
    set2DElement(array, 0, 4, 200);

    value = get2DElement(array, 0, 4);
    printf("Retrieve value %d from [0,4]\n\n", value);

    printf("Store value 3 at [2,2].\n");
    set2DElement(array, 2, 2, 3);

    value = get2DElement(array, 2, 2);
    printf("Retrieve value %d from [2,2]\n\n", value);

    free2DArray(array);



    printf("Testing Part 2\n");
    stringPtr = createStringArray(100);

    printf("Store string - fred\n");
    setStringArray(stringPtr, 44, "fred");
    printf("Store string - barney\n");
    setStringArray(stringPtr, 80, "barney");

    printf("Get string - %s\n", getStringArray(stringPtr, 44));
    printf("Get string - %s\n", getStringArray(stringPtr, 80));
    printf("Get string - %s\n\n", getStringArray(stringPtr, 3));

    freeStringArray(stringPtr, 100);



    printf("Testing Part 3\n");
    matrix = createArray(100, 100);

    printf("Store 33 44 55\n");
    matrix[22][76] = 33;
    matrix[83][29] = 44;
    matrix[99][65] = 55;

    printf("Retrieve %d %d %d\n", matrix[22][76], matrix[83][29],
       matrix[99][65]);

    freeArray(matrix, 100);

    return(1);
}

Am I using malloc() or free() in some way that precludes the program working? Can anybody compile this code and get it to work all the way through?

you might want to add some debugging print statements in some of those functions to see if they are writing beyond the boundries of the allocate memory for those arrays. For example, does loc_x + wd*loc_y exceed the allocation limits of the array?

> int ht, wd;
You use these in your array subscript calculation, but you never set them.
They should be passed as a parameter to the function anyway.

> stringPtr[position] = malloc(strlen(setstring) * sizeof(char));
When allocating a string, you need to count the \0 as well, otherwise it ends up trashing something. This is probably the cause of your problem.
stringPtr[position] = malloc( (strlen(setstring)+1) * sizeof(char));

True, but always writing malloc calls in this form p = malloc ( number * sizeof *p ); will ensure that the scaling for the size of object being allocated will always be correct.

http://c-faq.com/malloc/sizeofchar.html

from your own link

It's never necessary to multiply by sizeof(char), since sizeof(char) is, by definition, exactly 1.

p = malloc ( number * sizeof(char) ); is the same thing as p = malloc ( number * 1 ); which is identical to p = malloc ( number ); . So why bother with the extra typing :eek: and extra reading :eek:

One problem (I know you are aware of this but others may not) with using malloc(), of course, is that when the value of number is positive and greater than the maximum value that can be stored in size_t, which will happen only if sizeof(int) is not the same as sizeof(long). Or when the value of number is a negative signed int, in that case the value actually passed to malloc() may be unpredictable and probably undefined.

I don't see the point of thinking, "oh, this type is char, so I'll do this and save a few keystrokes" p = malloc( 10 ); Then later on thinking, "mmm, this isn't char, I need to scale it" p = malloc( 10 * sizeof *p ); > So why bother with the extra typing and extra reading
My point is, if there is exactly ONE way to do a particular job which is always correct, then that is the way you should do it. Because by habit that particular bit of code will always be correct.

If you have multiple choices, then sooner or later you're going to make a mess of it. On some of those occasions, the time taken to find the problem (hours, days, weeks) will far outstrip the time it takes to type in a few extra characters.

I can tell you that if you do spend days on finding such a bug, you won't be thinking "wow, that was a really interesting problem to solve". No, you'll be kicking yourself for making the same dumb-ass mistake yet again.

Comments
Amen, consistency and sound programming practice is the key - ~s.o.s~

So, did anybody figure out what was wrong with the code? I tried doing the "malloc(num * (stringsize + 1))" thing, but it still didn't work.

Never mind, I discovered the problem. It had to do with my freestringarray() function. I was trying to free all 100 strings, when in reality, I had only actually allocated two of them.

That's a good reason to initialize all pointers to NULL as soon as possible so that you can check their validity before attempting to free them. I believe that free() will do nothing if you send it a NULL pointer.

This question has already been answered. Start a new discussion instead.