Thread: Prog Hangs - Something To Do With Pointers

  1. #1
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218

    Prog Hangs - Something To Do With Pointers

    I'm making a substring function. When I call it the prog hangs and I don't understand why Heres my function:
    Code:
    int String::Sub(char *sub, bool case_sensitive=true)
    {
    	//Searches for a substring within the string
    	//Returns -1 for failure or the index of first occurence
    	bool in=false;
    	char *a='\0', *b='\0';
    	int found=-2; //if returns -2 then somethings wrong
    
    	for(int i=0; i<size; i++)
    	{
    		a=&str_ptr[i];	
    		if(!case_sensitive)
    			if(*a >= 'A' && *a <= 'Z') 
    				*a+=32;
    		
    		if(!in)
    		{					
    			b=sub;
    			if(!case_sensitive)
    				if(*b >= 'A' && *b <= 'Z') 
    					*b+=32;
    
    			if(*a == *b)
    			{
    				found=i;
    				in=true;
    			}			
    		}
    		else
    		{
    			b++;
    			if(*b == '\0') return found;
    			
    			if(!case_sensitive)
    				if(*b >= 'A' && *b <= 'Z') 
    					*b+=32;
    
    			if(*a != *b) 
    			{	
    				in=false;
    				i=found+1;
    			}
    		}
    	}
    	return -1;
    }
    If I comment out the red section it works fine, but it won't do a non case sensitive comparison properly. The thing is it works fine for all the other instances where I convert to lower case. Also heres my main function with headers and stuff excluded:
    Code:
    int _tmain(int argc, _TCHAR* argv[])
    {
    	String something("Something\n");
    	something.Print();
    	something.Add("Else\n");
    	cout << something.Sub("ELSE", false);
    	cin.ignore();
    	return 0;
    }
    Anyone know why this is happening?

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    I don't know why your code isn't working right. Although this bit looks a bit suspicious:
    Code:
    i=found+1;
    . If you are unlucky, you'll keep going over the same bit of string infinitely.

    Some other comments:
    I would recommend that you don't change the actual case of the input string. Instead, extract a character from the string, then fold the character to lowercase (if relevant). Also, use "tolower()" - it's more portable and may even work for foreign character sets.

    Similarly, it is quite unnecssary to take the address of the current character into a pointer a, when all you really need is a copy of the char str_ptr[i].

    There's quite a bit of duplication between your if/else branches - it is always best to avoid duplicating code.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    Oh yeah I'm an idiot. I forgot that using pointers is going to change the contents o_0
    I guess I'll start over. I think the i=found+1 but should never get stuck, because its always moving forward, but it might not be the best way to do this.

    Cheers.
    Last edited by mike_g; 09-19-2007 at 09:04 AM.

  4. #4
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    >> char *a='\0';

    that initialization seems a bit confused...

    >> *b+=32;

    you are modifying the callers string, which happens to be, in this case, a const literal. write the character to a temporary before comparison.
    Last edited by Sebastiani; 09-19-2007 at 08:55 AM. Reason: slow response
    Code:
    #include <cmath>
    #include <complex>
    bool euler_flip(bool value)
    {
        return std::pow
        (
            std::complex<float>(std::exp(1.0)), 
            std::complex<float>(0, 1) 
            * std::complex<float>(std::atan(1.0)
            *(1 << (value + 2)))
        ).real() < 0;
    }

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Sebastiani View Post
    >> char *a='\0';
    Nah, that's just a different way of saying
    Code:
    a = NULL;
    - ok, so it may not be perfectly compatible types, but ignoring warnings, it's fine. - not very good style, tho'.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #6
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    Okay I got it working now and im using tolower() as that should be better.

    >> char *a='\0';

    that initialization seems a bit confused...
    Thats the only way I know that I can get it declared and initialized in one line. And I don't get any warnings from it.

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by mike_g View Post
    Thats the only way I know that I can get it declared and initialized in one line. And I don't get any warnings from it.
    What compiler are you using, and what warning levels do you have enabled? It is definitely not "right" to do that. Try something like:
    Code:
       char *a = 0;
    But then you don't really need char *a if you follow the advice about using a temporary variable. You can just do something like:
    Code:
       char a;
       ...
       a = str_ptr[i];
       ...
       if (a == *b)
         ...
    Another point: Add braces at least for your if-statements that have more than one line inside them - it prevents you from getting strange effects when you decide to add a debug statement, such as:
    Code:
       if (something) 
           cout << "something is true";
           do some stuff;
    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  8. #8
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    What compiler are you using, and what warning levels do you have enabled? It is definitely not "right" to do that. Try something like:
    Code:

    char *a = 0;
    I get no warnings with that either. Is it okay to do that instead? i'm using VC++2005 with default warining levels. Maybe I'll see if I can turn it up.

    Another point: Add braces at least for your if-statements that have more than one line inside them - it prevents you from getting strange effects when you decide to add a debug statement, such as:
    Yeah I know, its a bad coding habit of mine.

    Oh and you were right about the i=found+1 thing being wrong, because I forgot i gets incermented at the start of each loop. But I still don't reckon it can get stuck in the loop.

    Cheers.

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by mike_g View Post
    Oh and you were right about the i=found+1 thing being wrong, because I forgot i gets incermented at the start of each loop. But I still don't reckon it can get stuck in the loop.

    Cheers.
    Yes, I guess you are right there.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. pointers to constants and constant pointers
    By homeyg in forum C++ Programming
    Replies: 1
    Last Post: 06-18-2005, 12:02 AM
  2. Arrays, pointers and strings
    By Apropos in forum C++ Programming
    Replies: 12
    Last Post: 03-21-2005, 11:25 PM
  3. pointers
    By InvariantLoop in forum C Programming
    Replies: 13
    Last Post: 02-04-2005, 09:32 AM
  4. Replies: 4
    Last Post: 09-12-2001, 02:05 PM