• Advertisement
Sign in to follow this  

Cant figure out why my variable is getting clobbered

This topic is 4340 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have a char pointer that get filled with a string and I also have a stl string that gets filled with some data. When I assign the stl string its data the char* suddenly becomes garbage and I can figure out why. Here is a small snippet where the problem occurs
	char *title = NULL;
	char *tm = NULL;
	title = idxgetTitle(temp); // copy the html file title in to the title variable...THIS WORKS WITHOUT A PROBLEM

	if(title != NULL)
	{
		//title = idxcheckTitle(title);

		std::string pTitle = "";   <----- title becomes garbage here
		pTitle = "	<LI> <OBJECT type=\"text/sitemap\">\n";



Can anyone point out my problem?? Jon

Share this post


Link to post
Share on other sites
Advertisement
Quote:
Original post by jon723
title = idxgetTitle(temp);


is this supposed to be assignation?

Share this post


Link to post
Share on other sites
Thats just a utility function I have that checks the string for some unwanted characters (it returns itself anyway). I commented that out while I was debugging since it isnt the cause of the problem.

Share this post


Link to post
Share on other sites
Quote:
Original post by jon723

title = idxgetTitle(temp); // copy the html file title in to the title variable...THIS WORKS WITHOUT A PROBLEM



That doesn't copy anything; it makes the title pointer point to something returned by the function idxgetTitle(). The cause of the problem is extremely probably within that function. If, for example, you return a pointer to a local array (which is wrong, and EVIL), the memory it uses will be freed after the function returns, and will subsequently be overwritten by the new pTitle variable.

Share this post


Link to post
Share on other sites
But now you'll have to remember to free it... not good. Besides, even if you do that, you should use new[]/delete[]. Why not just return a std::string and stop worrying about unimportant low-level stuff?

Share this post


Link to post
Share on other sites
Quote:
Original post by Sharlin
But now you'll have to remember to free it... not good. Besides, even if you do that, you should use new[]/delete[]. Why not just return a std::string and stop worrying about unimportant low-level stuff?

fine, be all C++ guru like. see if i care [razz]

Share this post


Link to post
Share on other sites
actually, I modified the function to take a second argument which is a pointer to that char which gets allocated and filled inside the function. The array that gets passed to the functions gets a copy of what it needs and everything in the world is happy.

Share this post


Link to post
Share on other sites
But now you have an ownership problem - whose responsibility is freeing what's been allocated within the function?

I second using std::string

If you absolutely must use char* I would pass a preallocated buffer into the function rather than allocating the memory inside the function

Share this post


Link to post
Share on other sites
... If you're going to be creating a std::string anyway to hold your constant pTitle, then surely you should have no objection to using it for your actual *string processing* function.

By the way, please get rid of the habit of declaring variables with a 'default' initialization, and then using them later on. Declare them where they are used, and initialize them with the first value they take on. It's shorter (due to avoided redundancy), clearer (since you don't leave the maintenance programmer wondering why the default value isn't used) and safer (because you'll be more sure of scoping your variables properly).


std::string title = idxgetTitle(temp);
// no 'tm' here; wait for it to get used...
if (title != "") {
// Ok, there's no "null" for non-pointer objects to check, but you can usually
// either return a "null object" such as a string equivalent to "", or throw an
// exception if said null object is "valid".
std::string pTitle = "
  • <OBJECT type=\"text/sitemap\">\n";


  • BTW, please show this 'idxgetTitle'; I suspect I or someone else can show you a much cleaner implementation :)

    Share this post


    Link to post
    Share on other sites
    Quote:
    Original post by chowe6685
    But now you have an ownership problem - whose responsibility is freeing what's been allocated within the function?

    I second using std::string

    If you absolutely must use char* I would pass a preallocated buffer into the function rather than allocating the memory inside the function


    Generally a good plan. This covers both approaches I would take (the C++ and the C) I dislike creating blocks of memory in common functions. Better to scope down than up (not sure if this is an official manner of speaking of scope... It's just how I think of it.)

    Since mallocing within the function means that the lowest level call is allocating and assigning memory to a higher level you are now trusting that higher level to deal with the memory responsibly. If you were to assign the memory in the highest level function which requires the variable, however, now you have that function deal with both the creation and deletion of the variable. It then passes that variable down the line in the form of a reference or just by value and when all is said and done, there is no ambiguity about creating a block somewhere and deleting it in a completely different portion of the program.

    Basically, it's not a huge difference either way, it's just that one way you have things created and deleted in a central site and passed down the line where required and in another you have things created in the lowest level function and pray that the functions above you manage the deletion when necissary. As with anything, there are exeptions and I totally get that. This entire statement is just a general view I have on scoping and mem responsibility.

    However, as was mentioned if you are using C++ then you should use C++. Get the STL involved unless profiling has given you cause to believe that a string is too heavy for what you're doing with char arrays.

    Share this post


    Link to post
    Share on other sites
    Sign in to follow this  

    • Advertisement