C++ error trying to 'rewrite' char array

Question

I am trying to convert a string into all upper case using the code:

int client::get_upper(char*item_in)
{
    int k ;
    char * temp_str;
    int length = strlen(item_in);
    temp_str = new char [length+1];
    for(k = 0; k < length; ++k)
        temp_str[k] = toupper(item_in[k]);
    temp_str[k] = '\0';
    for(k = 0; k < length; ++k)
        item_in[k] = temp_str[k];
    return 0;
}

Yet when I attempt to do so I receive an access violation writing location xxxxxxxx from Visual Studio. This is for a class, so I am restricted from using actual strings.

Answer 1

Assuming that you call that code correctly, I think you have an off-by-one error with temp_str[k] = '\0';

Answer 2

When calling get_upper, did you pass a string literal? For example, is your calling code something like this:

char *mystr = "stackoverflow.com";
client.get_upper(mystr)?

That will most likely trigger an access violation in Visual Studio.

If this is the case, you can change the definition of mystr to:

char mystr[] = "stackoverflow.com";

Answer 3

We can't tell why you are getting an error without seeing how the function is being called.

My suspicion is that the error has to do with the calling context and what item_in is actually pointing to.

Jeremy Friesner's comments are spot-on - you have a leak and you don't need the intermediate buffer anyway if the goal is for this to be destructive.

Answer 4

Your code works if used properly (simplest way is to simply pass a local cstring like this):

char test[] = "stackoverflow.com";
client::get_upper(test); // client interpreted as a namespace

Now, your function is full of bad approaches, namely the redundant copy which is left unmanaged ( a memory leak).

Rewritten a bit:

int client::get_upper(char *item_in)
{
    unsigned int length = strlen(item_in);
    for(int i = 0; i < length; ++k)
        item_in[i] = toupper(item_in[i]);

    return 0;
}

If you want to experiment a bit, here something for you, just for fun:

int client::get_upper(char *item_in)
{
    int length = strlen(item_in);
    for(int i = 0; i < length; ++i)
        if((item_in[i] >= 97 && item_in[i] <= 122))
            item_in[i] = (int)item_in[i] - 32; 

    return 0;
}

And your error most likely comes from the fact that you're trying to push a dynamic array of characters which you probably didn't really think through. Simply use a local character string, a simple null-terminated array. You didn't really give as a lot to go on, so this is just guess work. All I can do is help you simplify your expression. Since the return value does nothing, consider applying it to something or switch to void.

Hope it helps.

Answer 5

The mere fact that you're not allowed to use the standard string class doesn't mean you have to write this as a monolithic function.

I'd write a simple function to do the transformation in-place. Then, if you need to support working on read-only strings, write another function that duplicates input (using yet a third function) and then does an in-place transformation on the duplicate.

char *duplicate(char const *input);

char *upper_str(char *input); // does in-place transformation

char *upper_str(char const *input); // duplicates, then transforms the duplicate