Do you guys know why the following code crash during the runtime?

char* word;
word = new char[20];
word = "HeLlo"; 
for (auto it = word; it != NULL; it  ){        
    *it = (char) tolower(*it);

I'm trying to lowercase a char* (string). I'm using visual studio.

Thanks

CodePudding user response：

You cannot compare it to NULL. Instead you should be comparing *it to '\0'. Or better yet, use std::string and never worry about it :-)

In summary, when looping over a C-style string. You should be looping until the character you see is a '\0'. The iterator itself will never be NULL, since it is simply pointing a place in the string. The fact that the iterator has a type which can be compared to NULL is an implementation detail that you shouldn't touch directly.

Additionally, you are trying to write to a string literal. Which is a no-no :-).

EDIT: As noted by @Cheers and hth. - Alf, tolower can break if given negative values. So sadly, we need to add a cast to make sure this won't break if you feed it Latin-1 encoded data or similar.

This should work:

char word[] = "HeLlo";
for (auto it = word; *it != '\0';   it) {
    *it = tolower(static_cast<unsigned char>(*it));
}

CodePudding user response：

You're setting word to point to the string literal, but literals are read-only, so this results in undefined behavior when you assign to *it. You need to make a copy of it in the dynamically-allocated memory.

char *word = new char[20];
strcpy(word, "HeLlo");

Also in your loop you should compare *it != '\0'. The end of a string is indicated by the character being the null byte, not the pointer being null.

CodePudding user response：

Given code (as I'm writing this):

char* word;
word = new char[20];
word = "HeLlo"; 
for (auto it = word; it != NULL; it  ){        
    *it = (char) tolower(*it);

This code has Undefined Behavior in 2 distinct ways, and would have UB also in a third way if only the text data was slightly different:

• Buffer overrun.
  The continuation condition it != NULL will not be false until the pointer it has wrapped around at the end of the address range, if it does.

• Modifying read only memory.
  The pointer word is set to point to the first char of a string literal, and then the loop iterates over that string and assigns to each char.

• Passing possible negative value to tolower.
  The char classification functions require a non-negative argument, or else the special value EOF. This works fine with the string "HeLlo" under an assumption of ASCII or unsigned char type. But in general, e.g. with the string "Blåbærsyltetøy", directly passing each char value to tolower will result in negative values being passed; a correct invocation with ch of type char is (char) tolower( (unsigned char)ch ).

Additionally the code has a memory leak, by allocating some memory with new and then just forgetting about it.

A correct way to code the apparent intent:

using Byte = unsigned char;

auto to_lower( char const c )
    -> char
{ return Byte( tolower( Byte( c ) ) ); }

// ...
string word = "Hello";
for( char& ch : word ) { ch = to_lower( ch ); }

CodePudding user response：

There are already two nice answers on how to solve your issues using null terminated c-strings and poitners. For the sake of completeness, I propose you an approach using c strings:

string word;           // instead of char* 
//word = new char[20]; // no longuer needed: strings take care for themseves
word = "HeLlo";        //  no worry about deallocating previous values: strings take care for themselves
for (auto &it : word)  // use of range for, to iterate through all the string elements      
    it = (char) tolower(it);

CodePudding user response：

Its crashing because you are modifying a string literal.

CodePudding user response：

there is a dedicated functions for this use strupr for making string uppercase and strlwr for making the string lower case.

here is an usage example:

char str[ ] = "make me upper";
printf("%s\n",strupr(str));


char str[ ] = "make me lower";
printf("%s\n",strlwr (str));