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?
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';
^^^