How to build toLower for string correctly

Question

I'm trying to perform case insensitive strcmp on full name string. I have a function to convert C-style strings to lowercase.

    char* toLowerCase(char* string){
    
        char *s = string;
        unsigned long int s_len = strlen(s);
        char sToLower[s_len];
    
        for (int i = 0; i < s_len; i++)
            sToLower[i] = tolower(s[i]);
        sToLower[s_len] = 0;
    
        s = sToLower;
        return s;
    }

but strcpy and strcat doesn't work. how to solve it?

        char fullName[1024] = "";
        strcpy(fullName, toLowerCase(p->data.firstName));
        strcat(fullName, toLowerCase(p->data.lastName));

after `char sToLower[s_len];` the elements of `sToLower` go from index `0` to `s_len - 1`. Specifically `sToLower[s_len]` does not exist. — pmg, Mar 28 '22 at 20:26
If you follow your `for` loop statement with a semicolon, the line after is not considered part of the loop. — Govind Parmar, Mar 28 '22 at 20:27
Your function is returning a pointer to a local array, leading to undefined behaviour. — printf, Mar 28 '22 at 20:32
Will it only ever be used to return a string that doesn't need to persist? A single-use return that can immediately be forgotten? And not thread safe? Then make the pointer that is returned static like strtok does. — Jerry Jeremiah, Mar 28 '22 at 20:35

Vlad from Moscow · Accepted Answer · 2022-03-28T21:20:02.027

The function toLowerCase is incorrect because it returns a pointer to a local array that will not be alive after exiting the function. So using this pointer to access memory will invoke undefined behavior.

char* toLowerCase(char* string){
//...
char sToLower[s_len];
//...
s = sToLower;
return s;
}

Moreover the size of the local array is specified incorrectly

char sToLower[s_len];

you need declare it at least like

char sToLower[s_len + 1];

to reserve memory for the terminating zero character '\0'.

Either you need to change the source string in place (but in this case the function may not be used with string literals) or you need within the function to create dynamically a new array and store there the source string converted to lower case.

For example

char* toLowerCase( const char* s )
{
    char *sToLower = malloc( strlen( s ) + 1 );

    if ( sToLower != NULL )
    {
        char *p = sToLower;
        while ( ( *p++ = tolower( ( unsigned char )*s++ ) ) != '\0' );
    }

    return sToLower;
}

You will need to free the allocated memory in the function for the returned string when the string will not be required any more.

Or the function can be declared with two parameters that specify the source array and the destination array like

char* toLowerCase( char *dsn, const char* src );

That is the user of the function must supply the destination array that will store the source string converted to lower case.

Or the calling convention needs to pass both the source string and the size of and a pointer to an output buffer where the result should go: `size_t toLower(const char *src, size_t size, char buffer[size])` where the return value is the length of the copied string — or some variation on that theme. — Jonathan Leffler, Mar 28 '22 at 21:02
@JonathanLeffler Thanks. I have updated the answer taking into account your comment. — Vlad from Moscow, Mar 28 '22 at 21:09
If the function uses `malloc` (or similar) to allocate memory, then the documentation of the function should clearly state that it is the responsibility of the caller to `free` the returned pointer after it is no longer needed. — printf, Mar 28 '22 at 21:16
I would definitely consider the "source string in place" approach unless I was sure that I needed two copies. — Neil, Mar 28 '22 at 22:36

जलजनक · Answer 2 · 2022-03-28T22:07:40.193

As pointed out already, a local variable inside a function seizes to exist outside function scope. You can either dynamically allocate memory for target-string(& free later) OR supply it during the call.

Since fullName is already available you can modify the toLowerCase() function as below:

#include <inttypes.h>

size_t toLowerCopy (char* dst, size_t dsize, const char* src) {
    if (!src || dsize < 2 || !dst) return 0L;

    const char* tmp = src;
    while (*tmp && --dsize)
        *dst++ = tolower (*tmp++);
    *dst = '\0';
    return (tmp - src);
}

It's also mindful of destination buffer-size, counting terminal \0 char. You can call it for fullName as:


char fullName[1024];

size_t copied = toLowerCopy (fullName, sizeof(fullName), p->data.firstName);
// handle if (copied == 0)
size_t first_len = copied;
if (copied) fullName [copied++] = ' ';  // first name was not empty (separator)
copied = toLowerCopy (fullName + copied, sizeof(fullName) - copied, p->data.lastName);
// handle if (copied == 0)
if (!copied) fullName[first_len] = '\0'; // second name was emtpy

score 0 · Answer 3 · edited Mar 30 '22 at 06:53

0

Thanks Vlad from Moscow for your answer. Here's my solution:

void toLowerCase(char* target, const char* source){
while ((*target++ = tolower((unsigned char)*source++)) != '\0');}

        toLowerCase(fullname, p->data.firstName);
        toLowerCase(lastName, p->data.lastName);
        strcat(fullname, lastName);
        printf("%s", fullname);

edited Mar 30 '22 at 06:53

Darth-CodeX

2,166
1
6
23

answered Mar 28 '22 at 22:07

Avshalom

31
4

How to build toLower for string correctly

3 Answers3