A function that changes all lowercase letters of a string to uppercase

Question

I am trying to write a function that changes all lowercase letters of a string to uppercase. Here is my code:

/**
 * string_toupper - This function will replace all lowercase letters in
 * the string pointed by str to uppercase
 * @str: The string that will be checked for lowercase letters
 *
 * Return: The resulting string str, where all the letters are uppercase
 */

char *string_toupper(char *str)
{
    int i;

    for (i = 0; *str != '\0'; i++)
    {
        if (str[i] >= 'a' && str[i] <= 'z')
            str[i] -= 32;
    }
    return (str);
}

And I tried it using:

#include <stdio.h>

int main(void)
{
    char str[] = "Hello World!\n";
    char *ptr;

    ptr = string_toupper(str);
    printf("%s\n", ptr);
    printf("%s\n", str);
    return (0);
}

But I get the following output:

Segmentation fault(core dumped)

My approach --> I will check the string if it has a lowercase letter. Then I will subtract 32 from the character if it matches to a lowercase character. I did this to make the character to uppercase, by subtracting 32 I am able to get the uppercase letter of the corresponding lowercase character I have found in the string.

But I am getting a Segmentation fault error, why is it happening?

Answer 1

There is a major mistakes in your code:

• the test in for (i = 0; *str != '\0'; i++) in function string_toupper is incorrect: it only tests the first character of str instead of testing for the end of string. As coded, you keep modifying memory well beyond the end of the string until you reach an area of memory that cannot be read or written, causing a segmentation fault. The code has undefined behavior. You should instead write:

    for (i = 0; str[i] != '\0'; i++)
  

• Also note that if (str[i] >= 'a' && str[i] <= 'z') assumes that the lowercase letters form a contiguous block in the execution character set. While it is the case for ASCII, you should not make this assumption in portable code.

• Similarly, str[i] -= 32; is specific to the ASCII and related character sets. You should either use str[i] = str[i] - 'a' + 'A'; which is more readable or use the functions from <ctype.h>.

Here is a modified version:

#include <stdio.h>

/**
 * string_toupper - This function will replace all lowercase letters in
 * the string pointed by str with their uppercase equivalent
 * @str: The string that will be checked for lowercase letters
 *
 * Return: The resulting string str, where all the letters are uppercase
 */

char *string_toupper(char *str) {
    for (size_t i = 0; str[i] != '\0'; i++) {
        if (str[i] >= 'a' && str[i] <= 'z')
            str[i] = str[i] - 'a' + 'A';
    }
    return str;
}

int main() {
    char str[] = "Hello World!\n";
    char *ptr;

    printf("before: %s\n", str);
    ptr = string_toupper(str);
    printf("result: %s\n", ptr);
    printf(" after: %s\n", str);
    return 0;
}

And here is a portable version of string_toupper():

#include <ctype.h>
#include <stddef.h>

char *string_toupper(char *str) {
    for (size_t i = 0; str[i] != '\0'; i++) {
        if (islower((unsigned char)str[i]))
            str[i] = (char)toupper((unsigned char)str[i]);
    }
    return str;
}

Answer 2

OP's key problem is well explained by Prithvish: wrong loop test.

// for (i = 0; *str != '\0'; i++)
for (i = 0; str[i] != '\0'; i++)

---

To help OP with How can I make my code work on every environment?, some thoughts for later consideration.

Future names

"Function names that begin with str, mem, or wcs and a lowercase letter may be added to the declarations in the <string.h> header." C17dr § 7.31.13

So do not code function names that begin str<lowercase> to avoid future collisions.

Indexing type

int i; is too narrow a type for long lines. Use size_t for array indexing.

Alternatively simply increment the pointer.

Test case with classification is...() functions

str[i] >= 'a' && str[i] <= 'z' is incorrect on systems where [a...z] are not continuous. (Uncommon these days - example EBCDIC).

Simplify with topper()

To convert any character to its uppercase equivalent:

str[i] = toupper(str[i]);

Use unsigned access

is..(x) and toupper(x) functions need unsigned char character values (or EOF) for x.

On soon to be obsolete rare non-2's complement systems, character string should be accessed as unsigned char to avoid stopping on -0.

---

Putting this together:

#include <ctype.h>

char *str_toupper(char *str) {
  unsigned char *ustr = (unsigned char *) str;
  while (*ustr) {
    *ustr = toupper(*ustr);
    ustr++;
  }
  return str;
}

Answer 3

By request, this is offered for education and debate.

Some workplaces or institutes insist on a particular style wrt curly braces, etc. I freelance...

Notice that the function name is not reproduced in a comment block. Bad habit that leads to satisfying supervisors with copy/paste of comment blocks that are WRONG and certainly misleading. Better to let the code explain itself by using conventional idioms and standard libraries.

#include <stdio.h>
#include <ctype.h>
#include <assert.h>

char *string_toupper( char *str ) {
    // Uppercase all lowercase letters found in 'str'.
    // Return str after processing.

    assert( str != NULL ); // Trust no-one, especially yourself

    // Alternative for()::  for( int i = 0; str[ i ]; i++ )
    for( int i = 0; str[ i ] != '\0'; i++ )
        str[ i ] = (char)toupper( str[ i ] ); // note casting.

    return str;
}

int main( void ) {
    char str[] = "Hello World!";

    // No further use? Don't store return value; just use it.
    printf( "%s\n", string_toupper( str ) );
    printf( "%s\n", str );

    return 0;
}

Answer 4

change the for loop condition to for (i = 0; str[i] != '\0'; i++) since it should check every index.

char *string_toupper(char *str)
{
    int i;
    for (i = 0; str[i] != '\0'; i++)
    {
            if (str[i] >= 'a' && str[i] <= 'z')
                    str[i] =(int)str[i] - 32;
    }
    return (str);
 }