Help with search program.

[dacbo]

I'm building a program that searches a tree for a name and then outputs their phone number. The program all works well I need to make it ignore the case of the search input when searching for a name.

I've done this by building a function to make strings lowercase and then calling this function when comparing the strings.

However, its all gone tits up and I've been raking my brains as to whats not right. Its something to do with the way I've calling my data structure i would suspect.

Any input would be greatly appreciated.

char *search( Tree *root, entry *what ) {
     if( root == NULL ) {
         return "Not Found";
     } else if( strcmp(makelower(what->name), makelower(root->thisnode.name)) == 0) {
         return root->thisnode.number;
     } else if( strcmp(makelower(what->name), makelower(root->thisnode.name)) < 0 ) {
         return search(root->left, what);
     } else {
         return search(root->right, what);
     }
}

char *makelower(char *s) {
     int n = strlen(s);
     while (n > 0) {
           if (s[n] <= 90 && s[n] >= 65) {
                    s[n] = s[n] - 32;
           }
     n++;
     }
     return strdup(s);
}

[cboard_member]

http://www.eternallyconfuzzled.com/tuts/bst1.html

There's quite a lot of info on BST's over three pages, happy reading!

[ZuK]

Do you know that the makelower() function modifies the passed string and returns a copy of the modified string. And you never free the string returned by makelower() ?
Kurt

[dacbo]

i'll give that article a read, cheers

and no I didnt know that I don't free the string - maybe i should do that?

[ZuK]

and no I didnt know that I don't free the string - maybe i should do that?

SInce you modify the original string it does not make much sense to duplicate the string with strdup(). But if you choose to do so you should free the duplicate.
Kurt

[dacbo]

have modified the program as you said which has fixed my problem but I'm now getting segmentation faults with my makelower function

am I calling and compairing s[n] in the right way?

[ZuK]

Sorry I did not look much at your makelower() function.
It's completely wrong

char *makelower(char *s) {
     int n = strlen(s);           // n points to the '\0' terminator
     while (n > 0) {              // you loop until n will overflow
           if (s[n] <= 90 && s[n] >= 65) { 
                    s[n] = s[n] - 32;
           }
     n++;
     }
     return strdup(s);
}

it should be somethong like this. I chose not to modify the passed string

char *makelower(char *s) {
     char * ret = strdup(s);
     int n = strlen(ret);
    int i = 0;
     while (i  < n) {
           if (ret[i] <= 90 && ret[i] >= 65) { 
                    ret[i] = ret[i] - 32;
           }
         i++;
     }
     return ret;
}

But generally it is not a good idea to allocate memory in a function. It's too easy to forget to free the returned pointer.
Kurt

[dacbo]

ahah, thanks very much for your help - thats done it

stupid mistake by the looks of it

[quzah]

You also shouldn't rely on hard coded numbers to represent your alphabet. If you want to compare it to a character, then do so:

if( foo[ x ] <= 'Z' && foo[ x ] >= 'A' )

The alphabet is not guaranteed to have 90 be letter foo all the time, and 65 be letter bar.


Quzah.

[Dave_Sinkula]

Nor are letters of the alphabet guaranteed contiguous.
Nor are lowercase and uppercase letters guaranteed to be 32 elements apart.
Which is probably why the functions isupper, tolower, etc. are standard.

[dacbo]

yeah makes sense, cheers