Htydjtk

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("%sn",hex);

//outputs : 6162636465666768696A6B6C6D6E6F707172737475767778797A

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("%sn",hex);



    free(hex);

}

Hope this helps!

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.

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 = '';

    return s - str;

}



int main(void)

{

    const char *str = "abcdefghijklmnopqrstuvwxyz", *str2 = "æôƌԹظⓐa";

    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.