HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Integer-to-hex string generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
hexstringintegergenerator

Problem

I am using this as a learning exercise primarily and want to ensure my code is optimal. The main thing is that it is reliable and I am made aware of any flaws.

Could it be made more efficient or more readable? How about style?

I thought about returning a char*, but then the caller would have to think to deallocate the returned string. I saw this as a problem, so I left to caller to allocate and deallocate. Is that the right decision? Any comments?

```
/* Generate hex string from integer. Odd number of characters must be preceded
with zero character
Eg. 9 becomes "09", 10 becomes "0A" and 16 becomes "10"
*/

#include

unsigned int num_hex_digits(unsigned int n) {
int ret = 0;
while(n) {
n >>= 4;
++ret;
}
return ret;
}

void make_hex_string_easy(unsigned int invokeid, char** xref)
{
int pad = 0;
int len = num_hex_digits(invokeid);
/ if odd number, add 1 - zero pad number /
if(len % 2 != 0)
pad = 1;

sprintf(*xref, "%s%X", pad ? "0" : "", invokeid);
}

void make_hex_string_learning(unsigned int invokeid, char** xref)
{
char p = xref;
int pad = 0;
int len = num_hex_digits(invokeid);
/ if odd number, add 1 - zero pad number /
if(len % 2 != 0)
pad = 1;

/ move to end of number string /
p+= len + pad - 1;

while(invokeid) {
int tmp = invokeid & 0xF;
if(tmp >= 4;

p--;
}

if(pad) {
*p = '0';
}
}

int main() {
unsigned int testdata[] = {~0, 1, 255, 256, 0xFFFE, 0xFFFF, 0x10000, 0xABC };
int sz = sizeof(testdata) / sizeof(int);
int i;

char test = (char) calloc (20, 1);
printf("Using sprintf method\n");
for(i = 0; i < sz; ++i) {
make_hex_string_eas

Solution

Nothing jumps out:

Rather than this:

if(tmp < 10)
        *p = tmp + '0';
    else 
        *p = tmp + 'A' - 10;


I would have used:

*p = tmp + (tmp < 10) ? '0' : 'A' - 10;


Even though you pass around a pointer to a pointer (for re-alloc I assume). You don't actually do any reallocation. So I would simplify the code and just pass a pointer.

void make_hex_string_easy(unsigned int invokeid, char** xref) 
                                                 ^^^^^^   I would just use char*


Note: A lot of common interfaces when you pass a NULL as the destination return how many characters it would have used in the buffer. This leads to a lot of C code that looks like this:

size_t  s = make_hex_string_easy(number, NULL); // assuming you changed your code to return a value.
char*   b = malloc(s+1);
make_hex_string_easy(number, b);


But Since you don't use dynamic buffers I would remove the calloc.

printf("Using sprintf method\n");
for(i = 0; i < sz; ++i) {
    char  test[50] = {0}; // inits to 0
    make_hex_string_easy(testdata[i], test);  // assuming you change to just a pointer
    printf("hex string of %#10x = \t%10s\n", testdata[i], test);
}

Code Snippets

if(tmp < 10)
        *p = tmp + '0';
    else 
        *p = tmp + 'A' - 10;
*p = tmp + (tmp < 10) ? '0' : 'A' - 10;
void make_hex_string_easy(unsigned int invokeid, char** xref) 
                                                 ^^^^^^   I would just use char*
size_t  s = make_hex_string_easy(number, NULL); // assuming you changed your code to return a value.
char*   b = malloc(s+1);
make_hex_string_easy(number, b);
printf("Using sprintf method\n");
for(i = 0; i < sz; ++i) {
    char  test[50] = {0}; // inits to 0
    make_hex_string_easy(testdata[i], test);  // assuming you change to just a pointer
    printf("hex string of %#10x = \t%10s\n", testdata[i], test);
}

Context

StackExchange Code Review Q#30579, answer score: 5

Revisions (0)

No revisions yet.