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!
printf()requirehexto have a trailing\0byte? \$\endgroup\$void shutDown(char * msg) { perror(msg); exit(EXIT_FAILURE); }\$\endgroup\$newStr, and I was unlucky the couple of tests that I run didn't fail. \$\endgroup\$