Thread: stack overflow error

True. I was thinking more of what it was supposed to do, then what it actually does.

Yeah it would. But it might be partially overlapping instead of discreet new occurrence. If that's OK.

Originally Posted by azarue

Code:

strstr(strstr(ptr, ","),",")

will return the first occurence of "," in a string.

Code:

strstr(strstr(ptr, ",")+1,",")

will return the 2nd occurence of "," in a string.

Wouldn't it ?

or will crash if there is no first occurence...

Originally Posted by vart

or will crash if there is no first occurence...

Which I was eventually going to point out if nobody else did. In fact I find this rather amusing:

Code:

                    if (strstr(strstr(ptr, ",")+1,",") != NULL)
                        // irrelevant code omitted
                    else
                    {
                        if (strstr(ptr, ",")!=NULL)

In the 'else' case the code 'acknowledges' that code in the 'if' case could produce a NULL, yet it was not handled.

but the logic of the code is correct. I've tested many strings and does sort them alphabetically.

I'm doubting that the logic of the code is correct because your logic of having "tested many examples therefore it is correct" is itself incorrect. You need to learn that in the absense of unit tests, you can pretty much assume that anything over ten lines of code is broken.
The presence of printf's through a function that has no business doing IO shows that you are in the middle of poor mans debugging, so the logic clearly isn't finalised. Pumping the code full of printfs often makes debugging harder due to bloating up the size of the code and making it not all visible at once etc. If you're using VisualStudio then you can use "When Hit" breakpoints to output variables to the output window as it runs, without even having to stop the program, or modify the code!
If that's not enough, your stack overflow is the definitive proof that the code is broken.

You've seriously underestimated the value of using meaningful-yet-concise variable names as you go. Substituting good names later on takes more time, not less.

You absolutely need a unit test framework for this. If you can't find anything that suits your needs, then just write your own (like I did). It can be as simple or as complicated as you like.
Next, don't just feed in random examples and see that they come out correct. Make up specific examples to test specific portions of the code. For the most part each test should be testing a specific case.
Keep all your tests not just until you think that the code is 'done', but keep them for the lifetime of the code itself.
Consider learning about TDD (Test Driven Development). Even just learning about it but not strictly doing it, makes you a better programmer IMHO.

Before we can help you rewrite this properly, we're going to need a good description, including some examples of what it is supposed to do.

I rewrote the code and it works well, except for one issue that I just can't understand.

free(input); - When I remove this line from the code everything works PERFECT. but when included, it sometimes crashes.

Can you help me and tell me what am I doing wrong this time ?

The code gets a string separated with commas, and prints the string sorted alphabetically.

For example - for a,d,b,c; it will return a,b,c,d.

Code:

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

void extract(char *str, char *result, int length) /* extract string from start to first comma. stored in *result */{
    /* copy string from start to length*/
    result = strncpy(result, str, sizeof(char)*length);

    /* make sure string has an end */
    *(result+sizeof(char)*length) = '\0';
}
int getLength(char *str) /* return length from start to first occurence of comma */ {
    /* check existence of comma in string */
    if (strstr(str,",") != NULL)
    {
        return strlen(str) - strlen(strstr(str, ","));
    }
    else
    {
        return 0;
    }
}
void sort(char *current, char *buffer, char *result)
{

    /*  temporary var to store first word */
    char *word;

    /*  flag for buffer use */
    int useBuffer = 0;

    /*  reset buffer */
    *buffer = '\0';

    /* set result */
    strcpy(result, current);

    /* iterate while stream contains comma*/
    while (strstr(current,",") != NULL)
    {
        /* allocate stream in memory */
        word = malloc(getLength(current)+1);

        /* extract first word from string*/
        extract(current,word, getLength(current));

        /* remove extracted word from current, including the comma*/
        current = strcpy(current, strstr(current,",")+1);

        /* compare two strings lexicographically */
        if (strcmp(word, current)<0) /* no replacment */
        {
            /* add compared word and comma to buffer */
            strcat(buffer, word);
            strcat(buffer, ",");

            /* flag to use buffer */
            useBuffer = 1;

        }
        else  /* replace */
        {

            if (useBuffer) /* when word isn't first comapred, use buffer  */
            {
                /* append the rest of string to buffer */
                strcat(buffer, current);

                /* copy whole string to current */
                strcpy(current, buffer);
            }

            /* append compared word to end of string */
            strcat(current, ",");
            strcat(current, word);

            /* call function again */
            sort(current, buffer, result);
        }

        free(word);
    }
}
int main()
{
    /* max number of chars */
    int max = 0;

    /* pointers */
    char *input;
    char *buffer;
    char *result;


    /* allocate stream in memory */
    input = malloc(sizeof(char)*(max));

    /* get max size from user*/
    scanf("%d\n", &max);

    /* store C string in a pointer to allocated stream, user input never greater than max */
    if (fgets(input, max, stdin) != NULL)
    {

        if (strchr(input, '\n') != NULL)
        {
             /* clear new line constant */
            *(input+strlen(input)-1) = '\0';
        }
        /* malloc buffer, length of input because it is 1 charchter shorter without \n char */
        buffer = malloc(strlen(input)+1);
        result = malloc(strlen(input)+1);

        sort((input), buffer, result);
        printf("%s\n",result);

    }

    free(result);
    free(buffer);
    free(input);
    /* return */
    return 0;
}

I'm surprised I didn't notice it earlier.

Code:

    /* allocate stream in memory */
    input = malloc(sizeof(char)*(max));

    /* get max size from user*/
    scanf("%d\n", &max);

You allocate the memory before you read in the max from the user. So you're allocating 0 bytes of memory!

Other than that, the code seems to run fine.