Run-Time Check Failure #2 - Stack around the variable 'g' was corrupted.

my main:

Code:

---

int main (void)
{
        char g[10]={'F','I','N','E'};
        strcat_s539(g, 10, "CHINA");
        printf("%s\n", g);
        system("pause");
}

---

my string concatenating function:

Code:

---

int strcat_s539 (char *dest, unsigned size, const char *source)
{
        int i; //variable counter
        int j=0; //counter for concatenating w/o nesting
        char *temp; //variable to hold a temporary array to copy from source to dest.
        temp = malloc(size*sizeof(source)); //allocate array for temporary
        if (dest=='\0' || source=='\0' || (strlen539(dest)+strlen539(source))>(size-1)) //test for NULL and size big enough
        return (1);
        else
        {
        for (i=0; i<=strlen539(source); ++i)
        {
                temp[i] = source[i];
        }
        for (i=(strlen539(dest)); i<=(strlen539(dest)+strlen539(source)); ++i, ++j)
        {
                dest[i] = temp[j];
        }
        return (0);
        }
        free(temp);
}

---

Perhaps someone can spot my problem?

strlen539 is just a handwritten function that does the same thing as strlen if anyone is wondering b.t.w. and the function actually returns the correct concatenated string: FINECHINA... its just that it brain damages the computer anyway so something is wrong somewhere.

Code:

---

for (i=(strlen539(dest)); i<=(strlen539(dest)+strlen539(source)); ++i, ++j)

---

Also you need to learn to indent your if/else blocks properly, and why are you allocating such a HUGE temp buffer (or for that matter, why do you need any temp buffer)?

Code:

---

temp = malloc(size*sizeof(source)); //allocate array for temporary

---

Quote:

---

Originally Posted by cpjust

Code:

---

for (i=(strlen539(dest)); i<=(strlen539(dest)+strlen539(source)); ++i, ++j)

---

You're starting to copy after the end of dest.

---

dest is g which is 9 characters big not including the null character, it contains FINE from initialization so strlen539(dest) is equal to 4.

Quote:

---

Originally Posted by Sparrowhawk

dest is g which is 9 characters big not including the null character, it contains FINE from initialization so strlen539(dest) is equal to 4.

---

Oops, I guess I posted to quickly. I hilighted the real problem.

Quote:

---

Originally Posted by cpjust

Code:

---

for (i=(strlen539(dest)); i<=(strlen539(dest)+strlen539(source)); ++i, ++j)

---

Also you need to learn to indent your if/else blocks properly, and why are you allocating such a HUGE temp buffer (or for that matter, why do you need any temp buffer)?

Code:

---

temp = malloc(size*sizeof(source)); //allocate array for temporary

---

---

Doh just noticed I didn't need that temporary buffer at all you're right... As for the other highlight, I'm not sure what is being highlighted...

AFAIK

Code:

---

(strlen539(dest)+strlen539(source))

---

should be equal to 4+5 = 9. i<=9... so

dest[i] = temp[j] (using temporary buffer in this example just to be consistent with the post)

starts at

dest[4] = temp[0]

and iterates 5 times from 4 to 9 for i, and 0 to 5 for j.

I'm with sparrow, that looks right to me. I guess you can print some strlen539's to make sure they're right. But if the lengths are 4 and 5, then accessing dest[9] and source[5] is perfectly legit.

Wow, I'm really batting 1000 tonight! :)
I guess that's my sign to go to bed.

I'll give it one more shot though.
Try changing:

Code:

---

char g[10]={'F','I','N','E'};

---

to this:

Code:

---

char g[10] = "FINE";

---

The error message is saying that your stack is being corrupted, so you're overflowing your buffer somewhere.

Quote:

---

Originally Posted by cpjust

Wow, I'm really batting 1000 tonight! :)
I guess that's my sign to go to bed.

I'll give it one more shot though.
Try changing:

Code:

---

char g[10]={'F','I','N','E'};

---

to this:

Code:

---

char g[10] = "FINE";

---

The error message is saying that your stack is being corrupted, so you're overflowing your buffer somewhere.

---

Didn't have any effect. Someone wanna try running this in a different compiler than my own? I am using Visual Studio 2005, sometimes it can do some weird stuff.

I eliminated the temporary buffer altogether and experimented with trying to make g bigger... e.g. g[11], g[12]... but I still get the same error there too...

strlen539 works fine I tried setting strlen(dest) etc to ints and printing them to make sure they were the values that I wanted them to be and they are.

What purpose does copying to temp serve?
Because you leak temp all over the place with this code.

> i<=(strlen539(dest)+strlen539(source));
Perhaps because you're modifying dest in the loop as well, that the loop is running out of control.

Quote:

---

Originally Posted by Sparrowhawk

Didn't have any effect. Someone wanna try running this in a different compiler than my own? I am using Visual Studio 2005, sometimes it can do some weird stuff.

---

No, the error is that you corrupt the stack. It is not the compiler's fault, and to be honest, no other compiler would complain because this is a runtime error which means you need a debugger to find it. I don't know if gdb actually finds such errors.

UPDATE:

Code:

---

        for (i=(strlen539(dest)); i<=(strlen539(dest)+strlen539(source)); ++i, ++j)
        {
                dest[i] = temp[j];
        }

---

As you add characters to dest, strlen(dest) will increase! Eventually you will go out of bounds.
You need to rethink your whole function.

First you must make sure the destination is at least the size of its current contents + the length of the source to be added + 1 (for the '\0').
Then you go ahead and find the end of the destination buffer and start adding the source.

Cool, at least my 2nd try was right, even though it was for a different reason than I first thought. ;)

I forgot to add that unless dest is zeroed out, it will also cause undefined behavior, since you will eventually replace the '\0' with something else and then strlen will be undefined. Which is also the case with the original buffer which wasn't \0 terminated!

Quote:

---

Originally Posted by Elysia

I forgot to add that unless dest is zeroed out, it will also cause undefined behavior, since you will eventually replace the '\0' with something else and then strlen will be undefined. Which is also the case with the original buffer which wasn't \0 terminated!

---

Actually that's what I thought when I suggested changing the way g was initialized, but one thing that occurred to me while brushing my teeth last night is that the original way that g was being initialized should set everything after the 'E' to 0 (I don't know the quote from the C standard, but I'm sure someone will post it) -- i.e. zero initialization...

So this:

Code:

---

char g[10]={'F','I','N','E'};

---

Should always be "FINE\0\0\0\0\0\0";

For some weird reason, you are right... it is null-terminated.

Quote:

---

Originally Posted by cpjust

(I don't know the quote from the C standard, but I'm sure someone will post it) -- i.e. zero initialization...

---

As you wish :)

Quote:

---

Originally Posted by C99 Section 6.7.8 from Paragraph 19

all subobjects that are not initialized explicitly shall be initialized implicitly the same as objects that have static storage duration.

---

And then objects with static storage duration are zero initialised.