Thread: strcmp+pointers=memory problems?

I'm going to go nuts here. I have some code that is comparing file-names and types within a tree (for traversal.) I've made sure that each piece of data is good data (I.E. the filename taken in and the filename from the tree are both correct. I've checked by cerr'ing the output.)

BUT, even with them being viable data, not being private members, and not accessing any data they shouldn't be (as far as I can tell), I'm still getting crashes from it.

Code:

struct nodeT 
{
    char *filetype;
    nodeT *left, *right;
    //stuff
};

...

void TST::search(char *filename, char *filetype)
{
    nodeT *newNode,*curNode;
    curNode = root;		             
//Data is still good here for both filename and the current node's filename

while (1)
{
    if (strcmp(filetype, curNode->filetype) < 0) //CRASH!
    {
    //do stuff
    break;
    }
...
}

I can't understand why it's crashing on such a simple comparison. Further, in other parts of the program (such as reading in the data) the string comparisons of dynamic arrays go very smoothly.

Should I be referencing my pointers or something?

I don't see any filename variable inside the struct. Also, how do you pass the value to filename variable? Using strcpy? My assumption is you haven't allocated the nodeT.filename yet.

Double post.

> char *filetype;
Well since you're crashing on a line where you would have to be responsible for your own memory management, my advice would be to start using std::string in place of messy char pointers.

@g4j31a5
Whoops. I was retyping instead of actually posting my code. It's been edited to reflect how it actually is. The real code is a cascading if for what is essentially a trinary tree

@Salem
I actually am not allowed to. The professor has us all build dynamic arrays of our own for all input, and all storage, in order to make us more proficient later on when dealing with data more directly in bits/bytes (..or so we're told )

I'd love to use std::string, but barring that, figuring out why the pointers have legitimate data but aren't working in this instance of strcmp is the goal. Oddly enough, I have similar code in my "addNode" function, but that data is encapsulated in a "file" class construct, and for whatever reason passes without incident, and compares just fine! But here.. can't explain why it crashes.

Thanks thus far. Hopefully this post makes it more clear why the above hasn't been helpful yet (not that I'm insinuating anything ).



EDIT:
As a side note, I don't suppose it's a problem, but I've allowed my file type to be as such: ".xls" ".exe" ".wmv" and similar. I assume strcmp handles punctuation fine, but if I'm wrong, I could remedy it.

where do you check that curNode is not null?

Code:

void TST::search(char *filename, char *filetype)
{
    nodeT *newNode,*curNode;
    curNode = root;		                           //keeps track of where we are in our tree
cerr << endl << filetype << endl << curNode->filetype << endl; //good
cerr << strcmp(filetype, curNode->filetype); //doesn't show anything

    while(1)
    {
        if (curNode == NULL)                    //then we never created such a file  
	{
            cout << "The specified filetype does not exist." << endl;
            break;
        }
        
        if (strcmp(filetype, curNode->filetype) < 0)
        {
            curNode = curNode->left;            //traverse the tree
            break;
        }

        if (strcmp(filetype, curNode->filetype) > 0)
        {
            curNode = curNode->right;           //traverse the tree
            break;
        }

        if (strcmp(filetype, curNode->filetype) == 0)
        {
            curNode->bst.search(filename);      //filetype exists! Now search the BST for the file
            break;
        }
    }//end while
}//end search

The root node is initially set to null before creation of the trees. If there was.. for some reason, never data added, this does indeed ensure the root node is not null. Further, as mentioned, the curNode->filetype outputs good data.

The cerr statements are just me trying to figure the thing out. I don't know why strcmp doesn't like char*s given that that's how it's defined.

My question boils down to, how can I have good data and get strcmp doesn't work?


EDIT:
Just to say, I'm using Dev C++, and the only way, at the moment, I can get my program window to stay up is to use a line that pauses it before the crash. No good data for errors is really coming out of it either, and the external VC debugger (that I don't think is all that spectacular) doesn't even catch it. The program dies. It died similarly to another memory access problem (which is what I assume this has to do with.)

1. you call strcmp 3 times - call it once, store the result and then - check it
2. why are you exiting the loop after going left or right? you are not yet at the and of the searching
4. instead of if if if use

Code:

if(res < 0)
{
}
else if(res > 0)
{
}
else
{
}

construct

Originally Posted by vart

1. you call strcmp 3 times - call it once, store the result and then - check it
2. why are you exiting the loop after going left or right? you are not yet at the and of the searching
4. instead of if if if use

Code:

if(res < 0)
{
}
else if(res > 0)
{
}
else
{
}

construct

1. Irrelevant to the discussion.
2. Whoops.
4. It's not a problem.

Are you content with wholly ignoring my question? The closest thing I've found on the internet is the possibility of a missing null-terminating character, but everything, everywhere else, works fine. This would suggest that the character is there and fine, especially since the space for the character is allocated just for that purpose.

Originally Posted by tabstop

Your null check is way way way way way way too late (you've already tried to follow the pointer twice before that check happens). IOW: If you ever try to find a file that doesn't exist, you will crash. Or at least it would, if you were either continuing the loop or calling the thing recursively.

Also I know that cerr is supposed to be unbuffered, but I would put endl at the end of that line anyway (the one you say doesn't print).

Suggestion that sounds sarcastic but isn't: go get code::blocks instead of Dev-C++. No need for keeping windows open, gdb comes with, and just in general a better experience.

The only check I'm doing on that pointer is for error checking. Those things will go away when the problem is figured out. The if statement obviously won't tell me why it's crashing, so I'm outputting before hand and pausing the runtime to check values.

I will use an endl just to make sure the cerr is giving me good info. But what I get is my input, and a completely null comparison. I get a black line. Obviously something odd is happening within Strcmp in this function.

The NULL comes when it should during normal program operation, and indeed, the program works correctly if you simply have an empty tree.



Are we going to bash assignment code or does someone know a legitimate answer as to why Strcmp would be giving poor output when the input is good data?

Originally Posted by blurrymadness

Are we going to bash assignment code or does someone know a legitimate answer as to why Strcmp would be giving poor output when the input is good data?

Because we know that doesn't happen. We have used strcmp many times. It does not give poor output when it gets good data. We know that "strcmp(filetype, curNode->filetype)" will not crash if filetype and curNode->filetype are good pointers. Therefore, either filetype or curNode->filetype are bad pointers. It's just that simple.

Since we don't have any other code to look at, that's all we can tell you. But one or the other of those pointers is bad (or points to bad data).

To prove it, I will make you a special offer: you can PM me, I'll send you an e-mail address, you can send me the code and I will find you the error.

OH AND EDIT: And did you ever fix the "this doesn't actually search for anything" problem yet? Right now, if the file you look for isn't the root, it will just end (since all the paths break, as mentioned upthread) without doing anything.

Still playing with it. oddly enough, I did get good output this time. The only change was the "endl" added to the cerr statement. I'm assuming that the compiler is then directly using the result where the function is called later.

EDIT: Yes. I have cerr statements littering the whole program for diagnostic purposes. It's nice to see you caught it beforehand, but I would ended up seeing it. I understand my attitude is not warm and friendly in the previous post, but I've seen this error posted elsewhere, as stated, related to '\0', and I expected this problem had been heard of before by one of the big wigs on here.

Originally Posted by blurrymadness

the external VC debugger (that I don't think is all that spectacular) doesn't even catch it.

It doesn't catch things by default. In the debugger, go to Debug --> Exceptions and check the box that says "Win32 Exceptions."

If you don't have the full Visual Studio installed, I'd recommend using WinDbg instead of the VC debugger.