strToHex ( string to its hex representation as string)

Question

I want to convert strings to their hex representations as strings too (like hex dump programs), for example "abz" to "61627A".

char * strToHex( char * str )
{
 int length = strlen ( str );
 char * newStr = malloc( length * 2 );
 if ( !newStr ) shutDown ( "can't alloc memory" ) ;
 for ( int x = 0; x < length; x++){
 char y = str[ x ];
 sprintf ( newStr + x * 2, "%02X", y );
 }
 return newStr;
}

ShutDown definition is omitted here, it is a function that calls perror and exit()

I designed strToHex to be used like

char * str = "abcdefghijklmnopqrstuvwxyz";
char * hex = strToHex(str);
printf("%s\n",hex);
//outputs : 6162636465666768696A6B6C6D6E6F707172737475767778797A

• pacmaninbw: I'd be really interested to see what shutdown(char* msg) does.
• Neil: In the use case that was provided, since you can effectively predict the size, I would think it would be more natural to have a string buffer and the size passed in instead of creating it dynamically.
• jochen: Won't printf() require hex to have a trailing 0円 byte?
• Accountant م: @pacmaninbw The argument name is actually "msg" as you guessed 😂 . void shutDown(char * msg) { perror(msg); exit(EXIT_FAILURE); }
• Accountant م: @jochen Yes, thank you, I forgot to terminate newStr, and I was unlucky the couple of tests that I run didn't fail.

Answer 1

Bug

As Carsten points out, you need to allocate \$(\text{length}\cdot 2)+1\$ bytes, rather than \$(\text{length}\cdot2)\$ to account for the null terminator sprintf() adds.

Formatting

Most C formatting guides do not include spaces around the arguments to function calls, nor the expressions within an if-statement. For an example of a C style most C programmers would find acceptable, see OpenBSD's style(9) manual.

I choose to associate * with the variable name, rather than floating between the type and name. This disambiguates the following example:

int *a, b;

Here, a is a pointer to an integer, but b is only an integer. By moving the asterisk next to the name, it makes this clearer.

int length = strlen ( str );
char * newStr = malloc (length * 2 );
if ( !newStr) shutDown ( "can't allocate memory" ) ;

Becomes:

int const len = strlen(str);
char *const new_str = malloc(1 + len * 2);
if (new_str == NULL) {
 shutDown("can't allocate memory");
}

Error checking

Rather than calling shutDown() and exit()ing the program, you should instead return an error value which can be checked by the caller of str_to_hex(). Because you return a pointer, you can return NULL to indicate an error occurred and the caller should check errno.

Likewise, on some systems your program can incorrectly exit when length == 0. If we look at the manual page for malloc(3):

Return Value

The malloc() and calloc() functions return a pointer to the allocated memory that is suitably aligned for any kind of variable. On error, these functions return NULL. NULL may also be returned by a successful call to malloc() with a size of zero, or by a successful call to calloc() with nmemb or size equal to zero.

So by returning NULL we account for the case where malloc(3) returns NULL on success.

if (new_str == NULL) {
 shutDown("can't alloc memory");
}

Becomes:

if (new_str == NULL) {
 return NULL;
}

If you choose, you can also check if str is NULL before calling strlen(). This is up to you, and it's not uncommon in C to ignore this case and leave it as user error.

Looping

Use the size_t type in your loop rather than int. size_t is guaranteed be wide enough to hold any array index, while int is not.

Using i rather than x is more common for looping variables.

The y variable isn't needed. You can simply use str[i] in its place.

In terms of performance there's likely a faster option than using sprintf(). You should look into strtol(3).

Conclusion

Here is the code I ended up with:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char *
str_to_hex(char const *const str)
{
 size_t const len = strlen(str);
 char *const new_str = malloc(1 + len * 2);
 if (new_str == NULL) {
 return NULL;
 }
 for (size_t i = 0; i < len; ++i) {
 sprintf(new_str + i * 2, "%02X", str[i]);
 }
 return new_str;
}
int
main(void)
{
 char *str = "abz";
 char *hex = str_to_hex(str);
 if (hex == NULL && strlen(str) != 0) {
 /* error ... */
 }
 printf("%s\n",hex);
 free(hex);
}

Hope this helps!

• jochen: Won't printf() require hex to have a trailing 0円 byte?
• Carsten S: You should allocate 2*len+1 bytes.
• esote: sprintf adds a null terminator. @Carsten thanks, I forgot to include that, I'll update my answer
• Accountant م: Thank you very much I'll consider every point seriously. regarding the functions definition, you wrote the return type on a separate line then on the next line you continue the function like char * \nstr_to_hex(char const *const str)\n is this convention has a name or reference that I can refer to ?
• esote: @Accountantم It is from the OpenBSD manual, the style is called BSD kernel normal form (KNF). Around the middle of manual: "The function type should be on a line by itself preceding the function."

Answer 2

In my opinion, the most severe problem is "Insufficient target memory".

int length = strlen ( str );
char * newStr = malloc( length * 2 );

You are allocating twice the length of str, which is enough for all the hex characters (two hex chars per input byte).

But sprintf works different: "A terminating null character is automatically appended after the content" (see here).

So the last call to sprintf will write a terminating zero byte right after newStr, into unallocated memory. This might provoke all kinds of unintended behaviour, including (but not limited to) crashes.

• Accountant م: Yes thank you, it's a bug. I forgot to terminate newStr, thanks for highlighting this as it's the biggest problem in my code.

Answer 3

Just one addition: like asprintf vs snprintf. One can effectively predict the size, so I would think it natural to have a string buffer and the size passed in instead of creating it dynamically.

#include <stdlib.h> /* strtol */
#include <string.h> /* strlen */
#include <stdio.h> /* printf */
#include <assert.h> /* assert */
/** Converts {str} to the underlying bit representation in hex, stored in
 {hex}. It may fail to compute the entire string due to {hex_size}, in which
 case the return will be less then the {str} length.
 str: A valid null-terminated string.
 hex: The output string.
 hex_size: The output string's size.
 return: The number of characters from the original that it processed. */
static size_t strToHex(const char *str, char *hex, size_t hex_size)
{
 static const char digits[0x0F] = { '0', '1', '2', '3', '4', '5',
 '6', '7', '8', '9', 'A', 'B', 'C', 'E', 'F' };
 const size_t str_len = strlen(str), hex_len = hex_size - 1;
 const size_t length = str_len < hex_len / 2 ? str_len : hex_len / 2;
 const char *s = str;
 char *h = hex;
 size_t x;
 assert(str && hex);
 if(!hex_size) return 0;
 for(x = 0; x < length; x++)
 *h++ = digits[(*s & 0xF0) >> 4], *h++ = digits[*s++ & 0x0F];
 *h = '0円';
 return s - str;
}
int main(void)
{
 const char *str = "abcdefghijklmnopqrstuvwxyz", *str2 = "æôƌԹظaa";
 char hex[80];
 size_t ret;
 ret = strToHex(str, hex, sizeof hex);
 printf("\"%s\" -> \"%s\" (%lu.)\n", str, hex, (unsigned long)ret);
 ret = strToHex(str, hex, sizeof hex / 2);
 printf("\"%s\" -> \"%s\" (%lu.)\n", str, hex, (unsigned long)ret);
 ret = strToHex(str, hex, 0);
 printf("\"%s\" -> \"%s\" (%lu.)\n", str, hex, (unsigned long)ret);
 ret = strToHex(str2, hex, sizeof hex);
 printf("\"%s\" -> \"%s\" (%lu.)\n", str2, hex, (unsigned long)ret);
 return EXIT_SUCCESS;
}

It cannot really fail if given the proper input, so this simplifies error checking a lot, especially in C. malloc and sprintf are pretty slow functions, comparatively, so I expect this to be faster and more robust.

• Accountant م: Thank you very much, I like the way you documented the function, is this a known convention for documenting C code ? I also like the performance consideration that this function has over the function I posted, but I generally give easier user interfaces more priority over performance, unless I get bottlenecks. I will study your code today again with a deeper look.
• Neil: The key thing here is the absence of malloc, which makes it much easier to use. Not that malloc is bad, eg, asprintf, but I find it makes it more complex to use properly. I like to try to code so that it's easy to put into en.wikipedia.org/wiki/Doxygen, with some modifications for SO.

Answer 4

I did more tests on the function today and found another Bug (shame on me), and AFAIK on code review I can't change the original code in the question since it got reviews.

if there are bytes have values more than 127 it will be all displayed as FF by the function. To reproduce

char str[] = {127,0};
char * hex = strToHex(str);
printf("%s\n",hex); //pritnts 7F (NORMAL)
//now try with this
char str[] = {128,0};
char * hex = strToHex(str);
printf("%s\n",hex); //pritnts FF (BUG)

It appears if the function is used with non English characters because they are stored with the most significant bit is set 1 in UTF-8

The Fix

To Fix it, replace this line

sprintf ( newStr + x * 2, "%02X", y );

with this

sprintf ( newStr + x * 2, "%02hhX", y ); // added hh

This is because y is of type char or signed char and the X specifier expects the argument to be unsigned int if no length is provided, so we provided length hh to tell the function that X is unsigned char . Check the length table of printf.

If we didn't provided hh, the sprintf function is going to promote Y from signed char to unsigned int and this promotion will go like this

when we defined the str as char and assigned the value 128 to it, it's represented as

1000 0000

The compiler thought it is -128 because it's type is signed char, now function sprintf wants to promote it to unsigned int, so to represent -128 in size of int, it will be like

1111 1111 1111 1111 1111 1111 1000 0000
^^^^ ^^^^ ^^^^ ^^^^

and because we chose to show only 2 digits then we see the last 2 bytes FF.

more info are here , and here

• 1201ProgramAlarm: This is only an issue on implementations that treat char as signed. If a char is unsigned it isn't a problem. Other possible fixes include declaring y as an unsigned char, casting y to an unsigned char in the sprintf call, or masking it (y & 0xFF).