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?

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

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.

>> 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.

Quote:

---

Originally Posted by Sebastiani

>> 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

Okay I got it working now and im using tolower() as that should be better.

Quote:

---

>> 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.

Quote:

---

Originally Posted by mike_g

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

Quote:

---

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.

Quote:

---

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.

Quote:

---

Originally Posted by mike_g

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