Question

I'm trying to implement a function that receives a c string as input, converts all lowercase characters to uppercase, then stores the result in the output parameter. Here is the code for this function:

void makeUpper( const unsigned char* input, unsigned char* output ) 
{

    int inputLength = strlen((char*)input);
    int outputLength = strlen((char*)output);

    for (int i = 0; i < inputLength; i++)
    {
        if ((input[i] >= 97) && (input[i] <= 122))
        {
            output[i] = input[i] - 32;
        }
        else
        {
            output[i] = input[i];
        }
    }

}

Now, a problem will clearly arise in the case that inputLength > outputLength. To remedy this, I inserted a the following code between the inputLength & outputLength declarations and for loop.

if (inputLength > outputLength)
{
    for (int i = 0; i < (inputLength - outputLength); i++)
    {
        strcat((char*)output, " ");
    }
}

Not only is this producing an error (this function or variable may be unsafe...), but I'm almost certain that I'm going about this the wrong way. However, I cannot think of any alternatives.

EDIT:

The main function I am using is as follows:

int main() 
{

  unsigned char in[] = "HELLO aaaaaaaaaa 678";
  unsigned char out[] = "                    xxxxxxxxx";

  makeUpper( in, out );
  cout << in << " -> " << out << endl;
  makeUpper( out, in );
  cout << out << " -> " << in << endl;

 return 0;

}

What I the function should print is:

HELLO aaaaaaaaaa 678 -> HELLO AAAAAAAAAA 678xxxxxxxxx
HELLO AAAAAAAAAA 678xxxxxxxxx -> HELLO AAAAAAAAAA 678XXXXXXXXX
Was it helpful?

Solution

You are confusing the "present contents of the output parameter" with "available space". The former is irrelevant, and the only information you have about the latter is "there is at least this much space available".

Now it will depend on how the space for output was assigned in the first place. If you did something like

char output[100];
strcpy(output, "hello");

You would end up with space for 100 characters, but only 6 (5+1) actually used. You could therefore take the string "ThiS IS a STRING" and process it with your function, without problem.

But this is not safe, because you don't know how much space there is. The following approach would be better:

char *output;
output = malloc(100);

Now change your function prototype to

void makeUpper( const unsigned char* input, unsigned char** output ) 

and in your function, you do

inputLength = strlen(input);
*output = realloc(*output, inputLength + 1);

This will make sure that enough space is allocated for the output. Or you could return the value in the input vector - you already know there is enough space there...

EDIT In the example you give, there is enough space in the output; the question simply becomes one of "safely copying" the (uppercased) input to the output. In which case your function can look like this:

void makeUpper( const unsigned char* input, unsigned char* output ) 
{

    int inputLength = strlen(input);
    int outputLength = strlen(output);
    int ii;
    for (ii = 0; ii < inputLength; ii++)
    {
        output[ii] = toupper(input[ii]);
    }
    if(outputLength < inputLength) output[ii] = '\0';
}

The final line is there to make sure that if you increased the length of output (again, assuming this was memory you could validly access), then you still need to make sure there's a terminating nul character at the end of the string. In your example, you want the "rest of the output string" to still be there when the input is shorter than the output, so you need the if condition.

In general - if you do not know for sure that output is big enough, there is no way to make it bigger without having access to the address of the pointer - sometimes called the "handle".

OTHER TIPS

If the output buffer can be assumed to be created within the function:

// C++ version
void makeUpper(const unsigned char* input, unsigned char*& output)
{
    // assume output = null
    int inputLength = strlen((const char*)input);
    output = new unsigned char[inputLength + 1];
    memset(output, 0, inputLength + 1); // initialize the array to 0's

    for (int i = 0; i < inputLength; i++)
    {
        output[i] = ::toupper(input[i]); // why reinvent the wheel
    }
}

// C version
void makeUpper(const unsigned char* input, unsigned char** output) 
{
    // assume output = null
    int inputLength = strlen((const char*)input);
    *output = (unsigned char*)malloc((inputLength + 1) * sizeof(unsigned char));
    memset(*output, 0, (inputLength + 1) * sizeof(unsigned char)); // initialize the array to 0's

    for (int i = 0; i < inputLength; i++)
    {
        (*output)[i] = ::toupper(input[i]);
    }
}

output would need to be deleted/freed by whoever takes control of it.

If you do not want to properly size output inside the function:

void makeUpper(const unsigned char* input, unsigned char* output, unsigned int output_size) 
{
    // assume output != null, the current contents of output are irrelevant - you need it's size
    int inputLength = strlen((const char*)input);
    int maxLength = (inputLength < output_size - 1 ? inputLength : output_size - 1);
    memset(output, 0, output_size); // clear output

    for (int i = 0; i < maxLength; i++)
    {
        output[i] = ::toupper(input[i]); // why reinvent the wheel
    }
}

NOTE: The C++ tag was removed while I was writing my answer. This really only affects the first solution (as you would use malloc instead of new). I would shy away from realloc or calloc unless your requirements absolutely necessitate them.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top