CODEWITHSUNDEEP
cstringstring-comparison
Cory
Cory

Reputation: 245

Compare strings

Thanks! Works perfectly now. Java has made me stupid :(

I am having some difficulty comparing strings in C. I get correct output when I don't use my isMorse function, but when I use it the output becomes inaccurate and displays random characters. As far as I can tell, the variable "morse" is actually changed when strcmp is called on it. I am thinking that it has to do with "morse" not being a constant, but I am unsure of how to remedy it.

Thanks!!

char *EnglishToMorse(char english)
{
   static char *morse;

   int i;
   for (i = 0; i < LOOKUP_SIZE; i++)
   {
      if (lookup[i].character == english)
      {
         morse = lookup[i].morse;
         return morse;
      }
   }

   morse = &english;  // Problem was here!!!
   return morse;
}

Upvotes: 1

Views: 560

Answers (5)

Marlon
Marlon

Reputation: 20314

The reason your morse variable appears to change is because it points to an area on the stack. The reason it points to an area on the stack is because you assigned it the address of your parameter english, which got pushed onto the stack when you called your function then popped off the stack once the function completed.

Now your morse variable will point to whatever memory takes that same location on the stack, which will constantly change throughout the lifetime of your program.

In my opinion, the best way to fix this problem would be to return a NULL pointer from EnglishToMorse if the character is not A-Z... then check for the NULL pointer in your isMorse function. After all, it's good practice to check for NULL pointers in code.

char* EnglishToMorse(char english)
{
    int i;

    english = toupper(english);
    for (i = 0; i < LOOKUP_SIZE; i++)
    {
        if (lookup[i].character == english)
            return lookup[i].morse;
    }

    return NULL;
}

int isMorse(char* morse)
{
    int i;

    /* Check for NULL, so strcmp doesn't fail. */
    if (morse == NULL) return 0;

    for (i = 0; i < LOOKUP_SIZE; i++)
    {
        if(strcmp(morse, lookup[i].morse) == 0) 
            return 1;
    }

    return 0;
}

Upvotes: 0

Jim Balter
Jim Balter

Reputation: 16406

You have a static variable in EnglishToMorse, but it's the wrong one. There's no need for morse to be static -- you simply return it. But you do need english to be static -- rather than on the stack -- since you return its address. Also, it needs to be a NUL-terminated string. Do something like

char *EnglishToMorse(char english)
{
   static char save_english[2]; /* initialized to 0's */ 

   int i;
   for (i = 0; i < LOOKUP_SIZE; i++)
      if (lookup[i].character == english)
         return lookup[i].morse;

   save_english[0] = english;
   return save_english;
}

Note, however, that the caller of EnglishToMorse must use the result or save it before EnglishToMorse is called again, since the second call may overwrite static_english.

Upvotes: 1

Zan Lynx
Zan Lynx

Reputation: 54325

char *EnglishToMorse(char english)
and
morse = &english;
are the problem.

You should never return a pointer to a local variable or a function parameter.

Upvotes: 0

Greg Hewgill
Greg Hewgill

Reputation: 992737

It looks like the problem is likely in this function:

char *EnglishToMorse(char english) {
    static char *morse;
    // ...
    morse = &english;
    return morse;
}

You are returning the address of a parameter (english) that's passed into the function. This parameter ceases to exist after the function returns (and before the caller gets a chance to actually see the value). It appears as though you've attempted to fix this by declaring the morse variable static, but this only makes the morse variable itself static, not whatever it points to.

Also, strings in C must be terminated with a NUL character. By returning a pointer to a single character (as in english), there is no guarantee that the next byte in memory is or is not a NUL character. So the caller, who is expecting to see a NUL-terminated string, may get more than they bargained for.

Upvotes: 0

Heath Hunnicutt
Heath Hunnicutt

Reputation: 19457

I have a little guess. The function EnglishToMorse() might be returning a pointer to memory from the stack. If so, running another function after EnglishToMorse() will alter that memory. This would be due to a mistake in EnglishToMorse() -- declaring a local array of char and returning a pointer to it.

Without seeing the code for EnglishToMorse(), this is just a stab in the dark. You could provide us more code to look at, and win.

Upvotes: 3

Related Questions