Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The idea is that i want any size string to put the corresponding hex value into a byte array. I've seen a million ways to do it. Some of them didn't look to clean. My needs are speed over memory, so chose to try and implement a lookup table. Tell me what you think

const BYTE HEX[0x80] = //This is the ASCII table in number value form
{   //0     1     2     3     4     5     6    7      8     9     A     B     C     D     E     F
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//0
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//1
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//2
    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//3
    0x00, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//4
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//5
    0x00, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,//6
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 //7
};
void ByteUtil::StringToHex(const char* str, BYTE* hex)
{
    int len = strlen(str)/2;
    for(int i=0; i<len; i++)
    {
        hex[i] =  (HEX[*str++] << 4);
        hex[i] += (HEX[*str++]);
    }
}

This method assumes you've already figured out the length of the array. So to use it it would look somethign like this.

char* gky = "55D38577093A88F3B5EA40BBF11158813A2C662EB71FBAB9";
int len = strlen(gky)/2;
BYTE* GKY = new BYTE[len];
ByteUtil::StringToHex(gky, GKY);
ByteUtil::LogArray("GKY", GKY, len);
delete[] GKY;

this particular code snippet outputted this

2013-02-15.10:03:19 GKY(24)- 55:D3:85:77:09:3A:88:F3:B5:EA:40:BB:F1:11:58:81:3A:2C:66:2E:B7:1F:BA:B9:

share|improve this question
    
Looks to me like you're opening yourself up to potential sign-extension vulnerabilities. –  OJ. Feb 15 '13 at 23:58
    
@OJ. Although i have not heard about this vulnerability before (and was good research for myself) the likely hood of that happening in the program this is used for is very unlikely. The input is generated off of encrypted data using AES standards. The use of this is never converted to a useable number. meaning it is sent to a device for verification. everything revolving around this is strictly unsigned. Thank you though for you insight into this. –  Robert Snyder Feb 16 '13 at 0:29
    
No worries mate, glad you enjoyed the learning :) –  OJ. Feb 16 '13 at 9:40

1 Answer 1

up vote 2 down vote accepted

Some minor comments:

  • you are assuming there are no invalid chars in the input string.

  • why not pass the string length into the function; the caller has to know it in order to allocate the hex buffer.

  • len could be const

  • unnecessary brackets around RHS expressions in hex[i] assignments

  • I would prefer the second assignment to be |= not +=

  • it is usual to use uppercase names only for #defined constants

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.