1

I'm trying to convert character of a string to uppercase letters

int main (void)
{
    int i = 0;
    int n = 0;
    static char *str[] = { "wow",
                           "RACEcar", 
                           "No devil lived on.",
                           "rotor" };

    for(i = 0; i < strlen(*str); i++)
    {
        if(str[i] != NULL)
        {
            n = function(str[i]);
        }
    }
    return 0;
}

int function(char* x)
{
   int i = 0;
   int j = strlen(x);
   char c;
   for(i = 0; i < j; i++)
   {
      c = toupper(x[i]);
      x[i] = c;
   }

   return 0;
}

I got an error saying exc bad access, code 2 at line where x[i] = c; I'm not sure why I get this error, do I need to create another string and assign c to the new string? toupper return the uppercase version of the character but didnt actually change the element itself, so I'm not sure what wrong with assigning the value return by toupper back to the element.

khoa_vo123
  • 29
  • 5
  • i == 0 ? should it be i = 0? – Frank Zhang Mar 17 '15 at 02:05
  • yeah that was my typo when i type the question here, but it's not that problem – khoa_vo123 Mar 17 '15 at 02:07
  • How did you call this function? Post that part of the code. – Yu Hao Mar 17 '15 at 02:09
  • 2
    How are you calling that function? Do you by any chance do something like `function("string");` ? – rici Mar 17 '15 at 02:09
  • 3
    This is not relevant to your question, but `strlen` is relatively expensive. C strings don't store their length; in order to compute the length of `x`, strlen needs to scan the entire string, counting characters until it hits a NUL. Your `for` loop forces that to happen on *every character*; that's a lot of scans. – rici Mar 17 '15 at 02:12
  • I posted the main function in which i used to call the function – khoa_vo123 Mar 17 '15 at 02:14
  • 5
    Change to `static const char *str[] = { "wow", ...` and now see why it does not work. You are trying to change constant data. – chux - Reinstate Monica Mar 17 '15 at 02:15
  • 2
    And this loop: `for(i = 0; i < strlen(*str); i++)` is performed as often as the number of letters in the first word in the array of strings. `"wow"` has fewer letters than the array has strings, so the last value will never be used. If "wow" were longer, you would probably try uppercasing random memory. – rici Mar 17 '15 at 02:28
  • 1
    Note that the `for` loop will only try to convert 3 strings. As written, it crashes on the first. You really need some variant of `for (i = 0; i < sizeof(str)/sizeof(str[0]); i++)` to convert all the strings. And you really need to define modifiable strings, such as: char str[][20] = { "…", … };`. – Jonathan Leffler Mar 17 '15 at 02:32

1 Answers1

2

Your code attempts to modify a string literal , which causes undefined behaviour.

The string "No devil lived on." is non-modifiable. To help the compiler catch the error you should declare the array as:

static char const *str[] = { "wow", // etc.

For historical reasons, the compiler has to let it pass without breaking compilation if you forget to include the const. But it is still an error nonetheless, and some compilers will warn anyway.

For gcc you can use the flag -Wwrite-strings to disable support for the historical case; this will cause an error message to be generated for your code as-is.

M.M
  • 138,810
  • 21
  • 208
  • 365