How to pass a string to a function and return the same string changed in C?

Question

I have a function that converts the content of strings from hexadecimal symbols to binary symbols. In my simple example I have used only two hex symbols a and b, and the same string is converted two times.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char * decode_hex(char hex[])
{
   static char bin[9];
   bin[0] = '\0';
   for (int i=0; hex[i]!='\0'; i++)
   {
        switch(hex[i])
        {
            case 'a':
                strcat(bin, "1010");
                break;
            case 'b':
                strcat(bin, "1011");
                break;
        }
    }
    return  bin;
}

int main()
{
   char *r = malloc(9);
   for (int i=0; i<2; i++)
   {
      r[0] = '\0';
      strcat(r, "ab");
      printf("Original string:  %s\n", r);
      r = decode_hex(r);
      printf("Converted string: %s\n\n", r);
   }
}

In the first pass, the function returns a correct converted string, but an empty string is returned in the second pass.

Original string:  ab
Converted string: 10101011

Original string:  ab
Converted string:

Can anybody tell me why this happens?

ADDITION

I know I can omit the problem using two separate strings:

int main()
{
   char h[3];
   char *r = malloc(9);
   for (int i=0; i<2; i++)
   {
      h[0] = '\0';
      strcat(h, "ab");
      printf("Original string:  %s\n", h);
      r = decode_hex(h);
      printf("Converted string: %s\n\n", r);
   }
}

But I was wondering if it is possible to reuse the string like

r = decode_hex(r)

And how to avoid memory leak?

Answer 1

The function decode_hex returns a pointer to the first character of the static array bin:

char * decode_hex(char hex[])
{
   static char bin[9];
   //...
   return  bin;
}

So after the first call of the function the pointer r defined in main is reassigned by the returned pointer

r = decode_hex(r);

As a result the program produces a memory leak because the address of the previously allocated memory is now lost.

In the second call these statements

  r[0] = '\0';
  strcat(r, "ab");

deal already with the static array declared in the function. And when the function is called it at once sets the first character of its static array to the terminating zero character '\0':

char * decode_hex(char hex[])
{
   static char bin[9];
   bin[0] = '\0';
   //...

So the array bin in this case contains an empty string and the parameter hex in turn points to this array with the empty string.

It is a bad idea to use a static array within the function and moreover with a fixed size equal to some magic number. Always try to write more general functions.

The function should dynamically create a new array where the binary representation of the passed string will be stored. The passed array to the function should be unchanged.

I can suggest the following function implementation as shown in the demonstration program below.

#include <string.h>
#include <ctype.h>

char * decode_hex( const char hex[] )
{
    size_t n = strlen( hex );

    char *bin = malloc( 4 * n + 1 );

    if (bin != NULL)
    {
        bin[4 * n] = '\0';

        for (size_t i = n, j = 4 * n; i-- != 0; )
        {
            unsigned char c = hex[i];
            c = toupper( c );

            if (c > '9') c = 10 + c - 'A';
            else c -= '0';

            for (size_t k = 0; k < 4; ++k)
            {
                bin[--j] = '0' + c % 2;
                c /= 2;
            }
        }
    }

    return bin;
}

int main( void)
{
    const char *hex = "ab";

    puts( hex );

    char *bin = decode_hex( hex );

    if (bin) puts( bin );

    free( bin );

    hex = "123456789abcdef";

    puts( hex );

    bin = decode_hex( hex );

    if (bin) puts( bin );

    free( bin );
}

The program output is

ab
10101011
123456789abcdef
000100100011010001010110011110001001101010111100110111101111

Note: At first I made a typo in this statement

if (c > 9) c = 10 + c - 'A';
       ^^^

Now the code is updated and the statement looks correctly like

if (c > '9') c = 10 + c - 'A';
        ^^^

Answer 2

Your problem was that you reassigned the output to the input, which made the 2nd loop iteration work on the static variable from the function, and caused the pointer to original malloc'd data to be lost, creating a memory leak.

Proposed solution:

You didn't specify it the data should be modified in place.

If you would like it to modify string in place, there should be a restriction on the input size, and the function signature could be void decode_hex(char *hex[]).

Here's a version that follows your function signature char *decode_hex(char hex[]) and doesn't modify in place: it has a function that returns a pointer to newly allocated memory. On the top level, r is allocated statically. Then bin is allocated dynamically and returned, so the user has to free it.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

// Returns a pointer to newly allocated data. Caller needs to free it after use.
char *decode_hex(char hex[])
{
   char *bin = malloc(9 * sizeof(char));
   bin[0] = '\0';
   for (int i=0; hex[i]!='\0'; i++)
   {
        switch(hex[i])
        {
            case 'a':
                strcat(bin, "1010");
                break;
            case 'b':
                strcat(bin, "1011");
                break;
        }
    }
    return bin;
}

int main()
{
   char r[9];
   char *converted;

   for (int i=0; i<2; i++)
   {
      r[0] = '\0';
      strcat(r, "ab");
      printf("Original string:  %s\n", r);
      converted = decode_hex(r);
      printf("Converted string: %s\n\n", converted);

      // Do something with the converted string
      // ...
      // Deallocate the memory
      free(converted);
   }
}

Output:

Original string:  ab
Converted string: 10101011

Original string:  ab
Converted string: 10101011

Answer 3

You asked how to pass a string to a function and return the same string changed. Vlad from Moscow did a good job explaining what you did wrong, and although his code provides a solution, it does not exactly return the same string changed, it returns a different string, which you must then not forget to free().

The way we usually pass a string to a function which is going to modify it is by passing a pointer to a buffer that holds the string to be modified and is large enough to also contain the result. For safety, we also pass the size of the buffer. Thus, your function prototpe becomes like this:

void decode_hex( char buffer[], int size )

So, buffer has a length, which can be discovered with int n = strlen( buffer ), and it must have a size of 4 * n + 1. In decode_hex you can either ASSERT that the size is at least 4 * n + 1 or, you can stop appending characters to it once you have emitted 4 * n + 1 characters.

For that you do not need to use malloc() at all; you can define a char buffer[1024]; in your main().

Answer 4

From all the good answers I have learned that the same string can't be passed to a function and returned again, because r=decode_hex(r) causes the pointer to original malloc'd data to be lost. I see there are two solutions to avoid the problem. Either I can use one string as input to the function and another for the decoded data, or the string can be modified in place.

Alternative 1 - Two different strings, one for input and one for output

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *decode_hex(char hex[], int size)
{
   char *bin = malloc(size);
   bin[0] = '\0';
   for (int i=0; hex[i]!='\0'; i++)
   {
        switch(hex[i])
        {
            case 'a':
                strcat(bin, "1010");
                break;
            case 'b':
                strcat(bin, "1011");
                break;
        }
    }
    return bin;
}

int main()
{
   int number_of_hex_symbols = 2;
   int hex_size = number_of_hex_symbols + 1;
   int bin_size = 4*number_of_hex_symbols + 1;
   char r[hex_size];
   char *b;

   for (int i=0; i<2; i++)
   {
      r[0] = '\0';
      strcat(r, "ab");
      printf("Original string: %s\n", r);
      b = decode_hex(r, bin_size);
      printf("Decoded string:  %s\n\n", b);
      free(b);
   }
}

Alternative 2 - String modified in place

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void decode_hex(char *s, int size)
{
   char bin[size];
   bin[0] = '\0';

   for (int i=0; s[i]!='\0'; i++)
   {
        switch(s[i])
        {
            case 'a':
                strcat(bin, "1010");
                break;
            case 'b':
                strcat(bin, "1011");
                break;
        }
   }
   s[0] = '\0';
   strcat(s, bin);
}

void main()
{
   int number_of_hex_symbols = 2;
   int size = 4*number_of_hex_symbols + 1;
   char r[size];
   for (int i=0; i<2; i++)
   {
      r[0] = '\0';
      strcat(r, "ab");
      printf("Original string: %s\n", r);
      decode_hex(r, size);
      printf("Decoded string:  %s\n\n", r);
   }
}