0

I'm trying to implement a function upperCase that takes a string and capitalises any characters without returning an extra string. Running the code below I get 'Segmentation fault (core dumped)' when the memcpy command runs. Am I using memcpy incorrectly?

The commented out printArrChar just loops through the array and outputs its values, which outputs the intended letters from charArray.

void upperCase(const char *s){
    int i=0;
    size_t len = strlen(s)+1;
    char charArray[len];
    while (*(s+i) != '\0'){
        if (*s >=97 && *s <= 122){
            charArray[i] = *(s+i) -32;
        }
        else
        {
            charArray[i] = *(s+i);
        }
        i++;
    }
    charArray[len-1] = '\0';
    // printArrChar(charArray, sizeof(charArray)/sizeof(charArray[0]));
    memcpy(s, &charArray, len);
}

Edit: Full code for context. https://pastebin.com/Hf7HH5yu

I realise I made a mistake declaring my function as taking a const but giving it a non-constant array, but removing the const doesn't seem to fix the error.

Thomas White
  • 15
  • 4
  • How are you calling it? – Eugene Sh. Nov 17 '20 at 19:45
  • Passing a string literal? – Weather Vane Nov 17 '20 at 19:45
  • Tip: Instead of *magical numbers* like 97, 122 and 32 just do `'a'`, `'z'` and `' '` respectively. It reads a lot better. Also instead of `*(s + i)` just do `s[i]`. – tadman Nov 17 '20 at 19:46
  • @WeatherVane In that case I would wonder where the OP expects to get the result in :) Oh, well, maybe they are passing a pointer to one. – Eugene Sh. Nov 17 '20 at 19:46
  • why not just do the modif by changing s without `charArray` ? – Ôrel Nov 17 '20 at 19:46
  • @EugeneSh. the local array is copied back to the source. – Weather Vane Nov 17 '20 at 19:47
  • If you're doing an in-place modification, just do an in-place modification. That temporary array doesn't help. – tadman Nov 17 '20 at 19:47
  • 1
    @WeatherVane Yeah, I meant if the usage is something like `upperCase("Hello")` – Eugene Sh. Nov 17 '20 at 19:48
  • 1
    You declared `s` as a `const char *`. That is a promise that the function will not change the contents `s`. Yet the function uses `s` as the destination of a `memcpy`. – user3386109 Nov 17 '20 at 19:48
  • perhaps it should be `memcpy(s, charArray, len)` – Chase Nov 17 '20 at 19:48
  • 1
    Copying back into a `const char*` is asking for trouble. – tadman Nov 17 '20 at 19:48
  • @EugeneSh. I meant: `char *str = "Hello"; upperCase(str);` I can't follow your comments sorry. – Weather Vane Nov 17 '20 at 19:49
  • @WeatherVane It's OK, nevermind. – Eugene Sh. Nov 17 '20 at 19:52
  • 1
    Sorry I can't get your point at all. You can't modify a string literal, why are you taking issue with this? – Weather Vane Nov 17 '20 at 19:52
  • If you want to uppercase the string _in-place_, you don't need `charArray`. But, you couldn't [shouldn't] use `const char *s` [you want: `char *s`] for that. If you want to pass back the uppercase string in a _different_ buffer, you want: `void upperCase(char *charArray,const char *s)` or [`char *charArray = malloc(strlen(s) + 1)`], you want `char *upperCase(const char *s)` and a `return charArray;` at the bottom. – Craig Estey Nov 17 '20 at 19:54
  • 3
    Well, from your "full code" - `char *myString = "this is a string in c";` - this is defining a string *literal*, which is read-only. `myString` is just a pointer to that read-only memory. – Eugene Sh. Nov 17 '20 at 20:03
  • 1
    You can fix the "full code" by declaring `myString` as an array, e.g. `char myString[] = "this is a string in c";` That will copy the characters of the string literal into an array that *can* be modified. – user3386109 Nov 17 '20 at 20:06

0 Answers0