strcmp+pointers=memory problems?

This is a discussion on strcmp+pointers=memory problems? within the C++ Programming forums, part of the General Programming Boards category; I'm going to go nuts here. I have some code that is comparing file-names and types within a tree (for ...

  1. #1
    Registered User
    Join Date
    Apr 2007
    Posts
    27

    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?
    Last edited by blurrymadness; 04-16-2010 at 12:33 AM.

  2. #2
    In the Land of Diddly-Doo g4j31a5's Avatar
    Join Date
    Jul 2006
    Posts
    476
    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.
    ERROR: Brain not found. Please insert a new brain!

    “Do nothing which is of no use.” - Miyamoto Musashi.

  3. #3
    In the Land of Diddly-Doo g4j31a5's Avatar
    Join Date
    Jul 2006
    Posts
    476
    Double post.
    ERROR: Brain not found. Please insert a new brain!

    “Do nothing which is of no use.” - Miyamoto Musashi.

  4. #4
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,558
    > 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.
    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.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  5. #5
    Registered User
    Join Date
    Apr 2007
    Posts
    27
    @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.
    Last edited by blurrymadness; 04-16-2010 at 12:51 AM.

  6. #6
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    where do you check that curNode is not null?
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  7. #7
    Registered User
    Join Date
    Apr 2007
    Posts
    27
    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.)
    Last edited by blurrymadness; 04-16-2010 at 01:58 AM.

  8. #8
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    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
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  9. #9
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by blurrymadness View Post
    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.
    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.

  10. #10
    Registered User
    Join Date
    Apr 2007
    Posts
    27
    Quote Originally Posted by vart View Post
    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.

    Quote Originally Posted by tabstop View Post
    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?
    Last edited by blurrymadness; 04-16-2010 at 04:43 PM.

  11. #11
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by blurrymadness View Post
    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.
    Last edited by tabstop; 04-16-2010 at 04:46 PM.

  12. #12
    Registered User
    Join Date
    Apr 2007
    Posts
    27
    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.
    Last edited by blurrymadness; 04-16-2010 at 04:49 PM.

  13. #13
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,243
    Quote Originally Posted by blurrymadness View Post
    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.
    Code:
    //try
    //{
    	if (a) do { f( b); } while(1);
    	else   do { f(!b); } while(1);
    //}

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by blurrymadness View Post
    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.
    Just to clarify: does this mean that you were missing a \0? (It is rare to do so, since there's not many input mechanisms that will allow you to do so, although "having to do things by hand" should have led us to "maybe not using fgets but doing things the hard way".) If so, I would have expected you to complain when the filenames were printed with large amounts of gibberish after them. If they printed normally, then most likely the \0 is there.

  15. #15
    Registered User
    Join Date
    Mar 2010
    Posts
    68

    it is either the pointer, or no null char

    hmm well. If your program does crash then there is a problem.
    The problem lies in the fact that ROOT=NULL

    you attempt to access a variable THROUGH A NULL POINTER in your second cerr statement,

    the following is bad!!!! curNode==NULL cannot do this curNode->filetype

    cerr << strcmp(filetype, curNode->filetype); //doesn't show anything

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. problems with strcmp
    By loopymoo26 in forum C Programming
    Replies: 6
    Last Post: 05-13-2009, 02:10 AM
  2. strcmp() \ flushing output
    By Brain Cell in forum C Programming
    Replies: 7
    Last Post: 01-17-2005, 12:28 PM
  3. seg fault with strcmp
    By samps005 in forum C Programming
    Replies: 2
    Last Post: 05-04-2003, 04:32 PM
  4. strcmp, any borders?
    By demonus in forum C Programming
    Replies: 3
    Last Post: 11-15-2002, 01:48 AM
  5. strcmp problems
    By Unregistered in forum C Programming
    Replies: 5
    Last Post: 07-14-2002, 07:17 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21