Advice required on my ascii to hex conversion C++

[andrewfaseuk]

Hi,

Before I start I have a very basic knowledge of C++ so please be kind
!

I need to convert a input string i.e. from argv[] in the form
'FFEEDD....' containing hex values into a real hex coded string.

For example the input string :

AABBCCDDEEFF

need to be converted to the same as the below definition:

unsigned char data[6] = "\xAA\xBB\xCC\xDD\xEE\xFF";

I've managed to do this but its probably not the best solution, my
code is:

char byte[2];
byte[2] = '\0';
unsigned char *data2 = static_cast <unsigned char*>
(malloc((strlen(argv[2]))/2+1));
int i;
int j=0;
long l;

for (i=0; i < strlen(argv[2])-1;i=i+2) {
byte[0] = argv[2];
byte[1] = argv[2][i+1];
l = strtol(byte,NULL,16);
data2[j++] = (char)l;
}
data2[j] = '\0';

argv[2] contains the info I want to convert and data2 is the output
char array. Is there an easier way than this ? am I being very
unefficent ?

Answers appreciated

 

[Maxim Yegorushkin]

Hi,

Before I start I have a very basic knowledge of C++ so please be kind
!

I need to convert a input string i.e. from argv[] in the form
'FFEEDD....' containing hex values into a real hex coded string.

For example the input string :

AABBCCDDEEFF

need to be converted to the same as the below definition:

unsigned char data[6] = "\xAA\xBB\xCC\xDD\xEE\xFF";

I've managed to do this but its probably not the best solution, my
code is:

char byte[2];
byte[2] = '\0';

You've overflowed your buffer here. It only has elements with indexes 0
and 1 and you write to element 2 which is out of bounds.

unsigned char *data2 = static_cast <unsigned char*>
(malloc((strlen(argv[2]))/2+1));
int i;
int j=0;
long l;

for (i=0; i < strlen(argv[2])-1;i=i+2) {

Here you call strlen() on each iteration. A better idea would be to
call it once before the loop.

byte[0] = argv[2];
byte[1] = argv[2][i+1];
l = strtol(byte,NULL,16);

Here strtol may go wild, since byte buffer has only two elements and
its not zero terminated.

data2[j++] = (char)l;
}
data2[j] = '\0';

argv[2] contains the info I want to convert and data2 is the output
char array. Is there an easier way than this ? am I being very
unefficent ?

You also don't check for invalid input here.

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

inline int is_hex(char c)
{
return (c >= '0' && c <= '9')
|| ((c | 0x20) >= 'a' && (c | 0x20) <= 'f')
;
}

inline unsigned char hex2bin(unsigned char h, unsigned char l)
{
h |= 0x20; // to lower
h -= 0x30;
h -= -(h > 9) & 0x27;
l |= 0x20;
l -= 0x30;
l -= -(l > 9) & 0x27;
return h << 4 | l;
}

int main(int ac, char** av)
{
if(ac < 2)
return EXIT_FAILURE; // wrong number of arguments

size_t nibbles = strlen(av[1]);
if(nibbles % 2) // odd number of nibbles
return EXIT_FAILURE;
nibbles /= 2;

unsigned char* buf = (unsigned char*)malloc(nibbles);
if(!buf) // out of memory
return EXIT_FAILURE;

for(size_t n = 0; n != nibbles; ++n)
{
if(!is_hex(av[1][2 * n]) || !is_hex(av[1][2 * n + 1]))
return EXIT_FAILURE; // bad input hex string
buf[n] = hex2bin(av[1][2 * n], av[1][2 * n + 1]);
}

for(size_t n = 0; n != nibbles; ++n)
printf("%02hhx ", buf[n]);
printf("\n");
}

 

[andrewfaseuk]

char byte[2];
byte[2] = '\0';

You've overflowed your buffer here. It only has elements with indexes 0
and 1 and you write to element 2 which is out of bounds.

Yes basic mistake, it was byte[3] and then I changed it to char[2] asi
could'nt remember is 2 meant 0,1 and 2 were available ....

unsigned char *data2 = static_cast <unsigned char*>
(malloc((strlen(argv[2]))/2+1));
int i;
int j=0;
long l;

for (i=0; i < strlen(argv[2])-1;i=i+2) {

Here you call strlen() on each iteration. A better idea would be to
call it once before the loop.

good idea ...

byte[0] = argv[2];
byte[1] = argv[2][i+1];
l = strtol(byte,NULL,16);

Here strtol may go wild, since byte buffer has only two elements and
its not zero terminated.

Well it I use data[3] it will be zero terminated wont it ? as i zero
terminate the array before the loop

data2[j++] = (char)l;
}
data2[j] = '\0';

argv[2] contains the info I want to convert and data2 is the output
char array. Is there an easier way than this ? am I being very
unefficent ?

You also don't check for invalid input here.

i've checking before this code snippet for valid input, just didnt post
it ! but thanks for pointing it out !

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

inline int is_hex(char c)
{
return (c >= '0' && c <= '9')
|| ((c | 0x20) >= 'a' && (c | 0x20) <= 'f')
;
}

inline unsigned char hex2bin(unsigned char h, unsigned char l)
{
h |= 0x20; // to lower
h -= 0x30;
h -= -(h > 9) & 0x27;
l |= 0x20;
l -= 0x30;
l -= -(l > 9) & 0x27;
return h << 4 | l;
}

any chance you could explain how this works ? i don't know what "|="
means not "return h << 4 | l;" to be honest the whole thing seems a bit
confuising for me anyway

Cheers

Andy

 

[Maxim Yegorushkin]

(e-mail address removed) wrote:

[]

any chance you could explain how this works ? i don't know what "|="
means not "return h << 4 | l;" to be honest the whole thing seems a bit

Check out your C/C++ textbook for operators.

confuising for me anyway

hex2bin function was cut from an optimized parsing code. The function
is a bit tricky to understand because it was deliberately coded to
avoid branching (conditional jumps) in generated code.

Basically, converting a hex digit into a hex nibble (4 binary digits)
algorithm looks like;

char xdigit; // hex digit to convert [0-9A-Fa-f]
xdigit = tolower(xdigit); // make it lowercase [0-9a-f]
xdigit -= '0'; // if it was a [0-9] digit, it's the value now
if(xdigit > 9) // if it was a [a-f] digit, compensate for that
xdigit = xdigit + '0' - 'a';

The original code is just an optimization of the algorithm.