Thread: stack overflow error

Hi all forum members, this is my first post, usually I'd always found an answer somewhere across the web but this time I just can't understand what is the problem. I hope one of you can help !

I'm trying to sort a string separated by commas alphabetically. so I've used recursion in order to iterate. each time the function will assign two pointers. and then, using strcmp It'll compare those strings. a replacment will be made strcmp () will return a value > 0.

I have to mention that it is a draft and I really don't mind regarding the resources. and important of all It does work ! but not always. for the string attached in the text file I'm getting a stack overflow error probably becuase I've used malloc().

I'm using redirection...

Thank you in advance, waiting for the light...

Code:

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

char *sort(char *ptr, char *start, char *buf)
{
    char *ptr2;
    char *ptr3;
    int compare = 0;

    /*printf("MAIN string is: %s and length is %d\n",start, strlen(start));*/
    printf("ttt\n");
    printf("string is: %s and length is %d\n",ptr, strlen(ptr));

    /* check if string contains at least one comma*/
    if ((strchr(ptr, ',')) != NULL)
    {
        printf("length of ptr: %d\n",strlen(ptr));

        /* allocate memory stream */
        ptr2 = malloc(strlen(ptr));
        ptr3 = malloc(strlen(ptr));

        printf("contains 2nd comma? %s\n",strstr(strstr(ptr, ",")+1,","));

        /* check if stream contains a second coma */
        if (strstr(strstr(ptr, ",")+1,",") != NULL)
        {
            printf("length to copy: %d\n",strlen((ptr)) - strlen((strstr(ptr,","))));
            /* ptr 2 in the first word to compare */
            ptr2 = strncpy(ptr2,ptr,strlen(ptr)-strlen(strstr(ptr, ",")));
            *(ptr2+strlen(ptr)-strlen(strstr(ptr, ","))) = '\0';

            /* ptr 2 in the first word to compare */
            ptr3 = strncpy(ptr3,(strstr(ptr, ","))+1,strlen(strstr(ptr, ",")) - (strlen(strstr(strstr(ptr, ",")+1,",")))-1);
            *(ptr3+ strlen(strstr(ptr, ",")) - (strlen(strstr(strstr(ptr, ",")+1,",")))-1) = '\0';

        }
        else
        {
            printf("no second comma - else\n");
            ptr3 = strncpy(ptr3, strstr(ptr, ",")+1, strlen(strstr(ptr, ",")));
            *(ptr3+strlen(strstr(ptr, ","))) = '\0';

            ptr2 = strncpy(ptr2, (ptr),strlen(ptr) - strlen(strstr(ptr, ",")));
            *(ptr2+strlen(ptr) - strlen(strstr(ptr, ","))) = '\0';
        }

        compare = strcmp(ptr2,ptr3);
        printf("ptr2 is %s ptr3 is %s and compare is %d\n",ptr2,ptr3, compare);
        if (compare>0)
        {
            printf("--SHOULD BE REPLACED--.\n");
            printf("MAIN string is: %s and length is %d\n",start, strlen(start));
            printf("length to start: %d length of replace: %d\n",strlen(start),strlen(strstr(start, ptr)));
            if (strstr(strstr(ptr, ",")+1,",") != NULL)
            {
                printf("MAIN string is: %s and length is %d\n",start, strlen(start));
                if (strlen(start) - strlen(strstr(start, ptr))>0)
                {
                    printf("length of compared strings is not equal\n");
                    printf("MAIN string is: %s and length is %d\n",start, strlen(start));
                    printf("buffer is %s\n",buf);
                    buf = strncpy(buf, start, strlen(start) - strlen(strstr(start, ptr)));
                    printf("MAIN string is: %s and length is %d\n",start, strlen(start));
                    printf("%s\n",buf);
                    *(buf+strlen(start) - strlen(strstr(start, ptr))) = '\0';
                    printf("MAIN string is: %s and length is %d\n",start, strlen(start));
                    buf = strcat(buf, ptr3);
                    printf("%s\n",buf);
                    buf = strcat(buf, ",");
                    buf = strcat(buf, ptr2);
                    printf("%s\n",buf);
                    if (strstr(strstr(ptr, ",")+1,",") != NULL)
                    {
                        printf("2 commas exists in string\n");
                        buf = strcat(buf, strstr(strstr(ptr, ",")+1,","));
                    }
                    else
                    {

                        if (strstr(ptr, ",")!=NULL)
                        {
                            printf("1 comma exists in string\n");
                            printf("%s\n",ptr);
                            printf("ptr %s\n",strstr(ptr, ","));
                            printf("start: %s\n",strstr(start, ptr));
                            buf = strcat(buf, strstr(start, ptr)+1);
                        }
                        else
                        {
                            printf("no commas exists in string\n");
                             buf = strcat(buf, strstr(ptr, ","));
                        }

                    }
                    printf("%s\n",buf);

                    *(buf+strlen(start)) = '\0';
                }
                else
                {
                    printf("length of compared strings is equal\n");
                    buf = strcpy(buf, ptr3);
                    printf("buffer is %s\n",buf);
                    buf = strcat(buf, ",");
                    buf = strcat(buf, ptr2);

                    printf("buffer is %s\n",buf);
                    buf = strncat(buf, strstr(strstr(ptr, ",")+1,","),strlen(strstr(strstr(ptr, ","),",")));
                    *(buf+strlen(ptr3)+strlen(ptr2)+strlen(strstr(strstr(ptr, ","),","))) = '\0';
                    printf("buffer is %s\n",buf);
                }
            }
            else
            {
                printf("There\n");
                buf = strncpy(buf, start, strlen(start) - strlen(strstr(start, ptr)));
                *(buf+strlen(start) - strlen(strstr(start, ptr))) = '\0';
                buf = strcat(buf, ptr3);
                buf = strcat(buf, ",");
                buf = strcat(buf, ptr2);
            }
                strcpy(start,buf);
                printf("buffer is %s\n",buf);
                sort(buf,buf,start);
        }

        printf("no problema\n");
        sort(strstr(ptr, ",")+1,start,buf);
    }

    return buf;
}
int main()
{
    int max = 0;

    char *ptr;
    char *buf;

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

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

    /* store C string in allocated stream */
    if (fgets(ptr, max, stdin) != NULL)
    {
        *(ptr+strlen(ptr)-1) = '\0';
        buf = malloc(strlen(ptr)+1);
        strncpy(buf, ptr,strlen(ptr));
        *(buf + strlen(ptr)) = '\0';
        printf("%s",sort(ptr, ptr, buf));
    }

    return 0;
}

A few notes:
- I see some mallocs. But I don't see any frees. Oops?
- You allocate memory using strlen. But that doesn't take into account for the '\0' char. Oops?
- Consider using array notation: x[y] instead of pointer notation *(x + y). It makes for cleaner code.

It looks like I could make the code more stable than it is now, I'll work on that. let's see if it'll work.

thanks.

Thanks, it's just code written in a rush.

*(buf+strlen(ptr3)+strlen(ptr2)+strlen(strstr(strs tr(ptr, ","),","))) = '\0';

but I still don't understand what's the problem with that line? what's the better way ?

I don't know...that does NOT strike me as code "written in a rush"; it strikes me as "look at how clever I am" code. Like some Perl programmers' code. Clarity makes far, far more sense than cleverness.

I thought there's a problem with the mechanism, or maybe a better application logic to achieve the same goal.

I thought it would be an easy task, and it was written in a huge rush !

I will clean everything up and make it more readable. after I'll do that I'll post my code again, if I'll have any more problems with it.

If not, thanks you for your time. and I believe I'll be more active from now on. that's post was my first in a long time !

I don't know...that does NOT strike me as code "written in a rush"; it strikes me as "look at how clever I am" code. Like some Perl programmers' code. Clarity makes far, far more sense than cleverness.

No I don't think I tried to show my wisdom. but you are right, Clarity makes far, far more sense than cleverness. I will follow!

Code that should go right in the trash, imo.

Your goals should be:

1) An accurate program
2) An efficient program
3) A clear or transparent, program

Take a few example strings, and see how YOU would do this job, with paper and pencil, and then mimic that, in your algorithm for this program.

String handling is NOT a strong point of C, that your program should be using to excess.

Take a few example strings, and see how YOU would do this job, with paper and pencil, and then mimic that, in your algorithm for this program.

That's a great tip. the code was written poorly, couldn't agree with you more ! but the logic of the code is correct. I've tested many strings and does sort them alphabetically.

the problem with the code is how it was written(which I took to my attention). and the fact it sometimes gets a stack overflow, which is probably a direct effect of the structure and writing of the code.

I'll revise and let you know..

Thanks everyone...

I'm not convinced this does anything

Code:

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

Once the substring is found, why find it again?

I'm not convinced this does anything

Code:

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

Once the substring is found, why find it again?

It doesn't, it will return the first comma in a string, It should be

Code:

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

and then I will get the 2nd comma in a string. or NULL of no 2nd comma exists.