Segmentation Error for Implementation of toUpper() in C

Question

I am attempting my own implementation of the toupper() function in C; my function should work for strings of any length. I am not sure why my compiler is throwing a segmentation error when I attempt to change the contents of the string as seen below. Is there any way around this error? (I have tried using arrays as opposed to pointers, but to no avail.) Note: isLetter() is a function I wrote that determines whether a character is an alphabetical character.

void toUpper(char *s){
while(*s != '\0'){
    if(isLetter(*s)&&(*s> 90)){     
        *s += ('a' - 'A');
    }
    s++;
}

I call the function like this:

char *s = "Hello";
toUpper(s);

Show us the calling code. Did you pass a string literal as a paramater? Modifiying string literals is undefined behavior. — cadaniluk, Sep 02 '15 at 05:15
Suggestion: #include and use isalpha('a'). Don't write roll your own functions when there are standard C functions that already do this. — B. Nadolson, Sep 02 '15 at 05:21
You should also do some basic defensive programming steps in your function e.g. checking whether `s !=NULL` and some max length since you are writing to the argument. — AndersK, Sep 02 '15 at 05:31
I likewise suspect a read-only literal. If it is tossing a seg-fault, it will be trivial to run in a debugger, letting the bullets spray in above the shoulder area, and check the call-stack and local vars once the event takes place. Short of that, old-school debug output to a console is darn near as effective. Either way, more research is required on *your* part. — WhozCraig, Sep 02 '15 at 05:48
Stop passing non-modifiable data, replace the if with `if (*s>='a' && *s<='z')`, and change `+=` to `-=`. — David Heffernan, Sep 02 '15 at 05:50
Thanks for the feedback guys. Here's my calling sequence: char *s = "Hello"; toUpper(s); — MathDude857, Sep 02 '15 at 05:52
@MattJohnson That's the bug then. Please read the linked duplicate. — Lundin, Sep 02 '15 at 06:37

score 3 · Accepted Answer · edited May 23 '17 at 11:58

char *s = "Hello";

String literals are non-modifiable. By an accident of history they are compatible with char*. Hence the runtime error that you encountered. If you enable warnings your compiler will tell you that you are making a mistake here.

More information here: Are string literals const?

Pass a modifiable string instead:

char s[] = "Hello";
toUpper(s);

Then you'll need to fix the bugs in your function. The body of the loop should read:

if (*s >= 'a' && *s <= 'z')
    *s -= ('a' - 'A');

Segmentation Error for Implementation of toUpper() in C

1 Answers1