Unexpected String Behavior C++

Question

I'm learning C++ on my own and am reading the book "C++ Crash Course" which has the following exercise:

3-2. Add a read_from and a write_to function to Listing 3-6. These functions should read or write to upper or lower as appropriate. Perform bounds checking to prevent buffer overflows.

Listing 3-6 that it references looks like this:

#include <cstdio>

int main(){

    char lower[] = "abc?e";
    char upper[] = "ABC?E";
    char* upper_ptr = upper;

    lower[3] = 'd';
    upper_ptr[3] = 'D';

    char letter_d = lower[3];
    char letter_D = upper_ptr[3];

    printf("lower: %s\nupper: %s\n", lower, upper);
}

The way I rewrote it is this:

char* write_to(char string[], int index, char replacement){
    if (sizeof(string) <= index){
        string[index] = replacement;
        return string;
    }
    return NULL;
};

char read_from(char string[], int index){
    if (sizeof(string) <= index){
        return string[index];
    }
    return NULL;
};

int main(){

    char lower[] = "abc?e";
    char upper[] = "ABC?E";
    char* upper_ptr = upper;

    lower[3] = 'd';
    upper_ptr[3] = 'D';

    char letter_d = lower[3];
    char letter_D = upper_ptr[3];

    printf("lower: %s\nupper: %s\n", lower, upper);

    char* changed_lower[] = {write_to(lower, 3, 's')};

    printf("changed lower: %s\nindex 3 of upper: %s\n",
        changed_lower, read_from(upper, 3));

The program runs, but gives me some warnings and the wrong outcome:

lower: abcde
upper: ABCDE
changed lower: 
index 3 of upper: (null)

I think the program should return something more like:

lower: abcde
upper: ABCDE
changed lower: abcse
index 3 of upper: D

Where am I going wrong?

Edit: I removed the warnings because people were hyper-fixating on those and not the real problem, which is that it doesn't give me the correct output.

Read this about the `sizeof` issue: https://stackoverflow.com/questions/1328223/when-a-function-has-a-specific-size-array-parameter-why-is-it-replaced-with-a-p — Retired Ninja, Jan 08 '22 at 18:41
Despite the use of a `.cpp` file extension and a C++ compiler, this code could pretty much compile as is in C. A book that teaches C as if it were C++ does not strike me as a good book. You should really be using `std::string`, but if you must keep using arrays, keep in mind that an array as a function parameter is actually a pointer in disguise, and therefore `sizeof` will not return what you expect for those. You should pass in the size as well in those cases, so it is not lost. — Etienne de Martel, Jan 08 '22 at 18:42
_"Where am I going wrong?"_ That's what your warnings are telling you. Are you hinting that you don't understand what the warnings are telling you? — Drew Dormann, Jan 08 '22 at 18:46
I haven't gotten to strings yet, I just used the varriable named string because the term is faimiliar to me — Sam L, Jan 08 '22 at 18:47
As pointed out, read about `sizeof`. It is a _compile time_ operator, it can not measure stuff at run time. If you insist on using *C* like code, use `strlen`. — lakeweb, Jan 08 '22 at 18:49
strlen did make two of the warnings go away but those weren't my biggest concern. It still doesn't print what think it should. — Sam L, Jan 08 '22 at 18:57
The fact that you don't feel the warnings were an issue is a problem in itself. If instead of `if (sizeof(string) <= index){` you used the proper order of `if (index < sizeof(string)){` you could easily access outside the bounds of the arrays you're using. Some other advice is to choose a name other than `string` for your variables. If this code was actual C++ and you'd picked up the bad habit of `using namespace std;` you'd likely have some interesting and not easily comprehensible to a beginner errors to deal with. — Retired Ninja, Jan 08 '22 at 21:53

Vlad from Moscow · Answer 1 · 2022-01-08T19:32:53.257

There are many errors.

For example this function declaration

char* write_to(char string[], int index, char replacement){

is equivalent to

char* write_to(char *string, int index, char replacement){

due to implicit adjusting the parameter having an array type to pointer to the array element type by the compiler.

So within the functions the expression

sizeof(string)

yields the size of a pointer instead of the size of the passed array.

Instead of the operator sizeof you need to use the standard C string function strlen declared in the header <cstring>.

Also it does not make a sense to return NULL from the both functions.

return NULL;

As the function read_from has the return type char then you may not use the conversion specifier %s with the function return value

printf("changed lower: %s\nindex 3 of upper: %s\n",
        changed_lower, read_from(upper, 3));

You need to use the conversion specifier %c.

Declaration of the variable changed_lower as having an array type

char* changed_lower[] = {write_to(lower, 3, 's')};

also does not make a great sense. You may not output it using the conversion specifier %s. At least if the variable has the array type you need to use the argument expression changed_lower[0] in the call of printf.

Though it is better to declare just a pointer

char *changed_lower = write_to(lower, 3, 's');

The functions can be declared and defined the following way

#include <cstring>

char * write_to( char string[], size_t index, char replacement )
{
    if ( index < std::strlen( string ) )
    {
        string[index] = replacement;
    }

    return string;
}


char read_from( const char string[], size_t index )
{
    return index < std::strlen( string ) ? string[index] : '\0';
}

Pay attention to that the null declarations after the function declarations like this

char* write_to(char string[], int index, char replacement){
    //...
};
^^^

are redundant and should be removed.

score 0 · Answer 2 · answered Jan 08 '22 at 18:49

0

void func1() {
    char str[] = "abcde";
    func2(str);
    ...
}

void func2(char str[]) {
    ...
}

If func1, the value of sizeof(str) is equal to 6 (5 characters + the null character).

If func2, the value of sizeof(str) is equal to sizeof(char*), i.e., the size of a memory pointer on your platform (typically 4 or 8 bytes, but can be any other value in theory).

When you call func2(str), only a pointer to the beginning of the string is passed to the function, not the entire string.

answered Jan 08 '22 at 18:49

But the variable can't be in the body of the function, because I need it to work on multiple items. I would need to pass it as a parameter, wouldn't I? – Sam L Jan 08 '22 at 18:53
@SamL: The point is that you should not use `sizeof(string)` when `string` is passed to the function from somewhere else. Instead, you should use `strlen(string)+1`. – Jan 08 '22 at 18:54
That does remove 2 of the warnings but doesn't change the behavior of the program, which is my primary question. – Sam L Jan 08 '22 at 19:03

Unexpected String Behavior C++

2 Answers2