patterncMinor
Integer-to-hex string generator
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
```
/* 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
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:
I would have used:
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.
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:
But Since you don't use dynamic buffers I would remove the calloc.
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.