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

  1. #1
    Registered User
    Join Date
    Oct 2008
    Posts
    98

    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.

  2. #2
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    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
    Last edited by cpjust; 11-16-2008 at 10:25 PM.
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  3. #3
    Registered User
    Join Date
    Oct 2008
    Posts
    98
    Quote Originally Posted by cpjust View Post
    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.

  4. #4
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by Sparrowhawk View Post
    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.
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  5. #5
    Registered User
    Join Date
    Oct 2008
    Posts
    98
    Quote Originally Posted by cpjust View Post
    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.

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    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.

  7. #7
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    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.
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  8. #8
    Registered User
    Join Date
    Oct 2008
    Posts
    98
    Quote Originally Posted by cpjust View Post
    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.

  9. #9
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    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.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  10. #10
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by Sparrowhawk View Post
    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.
    Last edited by Elysia; 11-17-2008 at 03:46 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  11. #11
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Cool, at least my 2nd try was right, even though it was for a different reason than I first thought.
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  12. #12
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    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 Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  13. #13
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by Elysia View Post
    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";
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  14. #14
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    For some weird reason, you are right... it is null-terminated.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  15. #15
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    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.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. how to put a check on an extern variable set as flag
    By rebelsoul in forum C Programming
    Replies: 18
    Last Post: 05-25-2009, 03:13 AM
  2. Run-Time Check Failure #2 help
    By lazysam in forum C Programming
    Replies: 2
    Last Post: 04-29-2009, 09:14 AM
  3. Replies: 3
    Last Post: 11-20-2008, 12:31 PM
  4. corrupted stack???
    By Abda92 in forum C Programming
    Replies: 12
    Last Post: 01-31-2008, 03:05 AM
  5. Stack functions as arrays instead of node pointers
    By sballew in forum C Programming
    Replies: 8
    Last Post: 12-04-2001, 11:13 AM