Hex conversion going terribly wrong in CPP

Question

I amtrying to read command line hex arguments in an unsigned char array. My code:

    #include <stdio.h>
#include <iostream>
#include <stdlib.h>
#include <memory.h>
#include <cstring>

unsigned char key[16] ={0x00, 0x00, 0x00, 0x00,
                                0x00, 0x00, 0x00, 0x00,
                                0x00, 0x00, 0x00, 0x00,
                                0x00, 0x00, 0x00, 0x00};


int main(int argc, char** argv){
        char hex_pref[]="0x";
        for (int i = 0; i < 16; i++){
                key[i] = strtol(strcat(hex_pref, argv[i]), NULL, 16);
                std::cout << argv[i] << "\t";
        }
        std::cout << std::endl;
        for (int i = 0; i < 16; i++)
                std::cout << key[i] << "\t";
        std::cout << std::endl;
        return 0;
}

The output of the program is very unpredictable. Everytime I change the arguments I get weird results.

I am running it like

./a.out ab 32 bf 00 0a 2e 4c 3d 25 db 66 22 84 fb 19 72

I tried debugging but no answer yet. Any suggestions? Thank you.

---

did this the following but problem persists as in I am not able to verify what key[i] has.

char hex_pref[4];
        strcpy(hex_pref, "0x");
        for (int i = 0; i < 16; i++){
                strcat(hex_pref, argv[i]);
                key[i] = strtol(hex_pref, NULL, 16);
                //std::cout << argv[i] << "\t";
        }
        std::cout << std::endl;
        for (int i = 0; i < 16; i++)
                std::cout << std::hex << key[i] << "\t";
        std::cout << std::endl;
        return 0;

---

one more failed attempt. now as one user said, key[i] prints garbage even with std::hex

for (int i = 1; i < 17; i++){
                //strcat(hex_pref, argv[i]);
                key[i-1] = strtol(argv[i], NULL, 16);
                //std::cout << argv[i] << "\t";
        }
        std::cout << std::endl;
        for (int i = 0; i < 16; i++)
                std::cout << std::hex << key[i] << "\t";
        std::cout << std::endl;
        return 0;

Answer 1

strcat() is trying to put the result into hex_pref which isn't prepared for that since it has size "2". That is never going to work right.

http://www.cplusplus.com/reference/cstring/strcat/

Keeping with your original approach. The code below is "correct". And produces good output (see below code).

#include <stdio.h>
#include <iostream>
#include <stdlib.h>
#include <memory.h>
#include <cstring>

unsigned char key[16] = { 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00 };


int main(int argc, char** argv)
    {
    char hex_pref[] = "0x";
    for (int i = 1; i < 17 && i < argc; ++i)
        {
        std::cout << argv[i] << "\t";
        if ( strlen(argv[i]) < 3)
            {
            char result[5];
            result[0]= 0;
            strcat(result, hex_pref);
            strcat(result, argv[i]);
            key[i-1] = char(strtol(result, NULL, 16));
            }
        }
    std::cout << std::endl;
    for (int i = 0; i < 16; ++i)
        {
        std::cout << int(key[i]) << "\t";
        }
    std::cout << std::endl;
    return 0;
    }

Output is:

C:\Users\rjohnson\Desktop\Workspaces\TestHex\Debug>testhex  ab 32 bf 00 0a 2e 4c 3d 25 db 66 22 84 fb 19 72
ab      32      bf      00      0a      2e      4c      3d      25      db      66      22      84      fb      19      72
171     50      191     0       10      46      76      61      37      219     102     34      132     251     25      114

A different approach using streams and strings would be the "real" way. There isn't really room here to provide a full explanation, but there are plenty of tutorials and full-blown examples out there.

string result;
result += "0x";
result += argv[i];
key[i]= strtol(result.c_str(), NULL, 16);

Answer 2

Problem 1:

argv[0] is not the first user provided argument. arg[0] is reserved for use by the implementation. Typically it is the command used to execute the program. In this case "./a.out". "./a.out" doesn't convert to an integer resulting in bad behavior since strtol is not being checked for validity.

Solution: start at argv[1] and increase the for loop accordingly.

Addendum: Don't ignore strtol's second parameter. It is very handy in picking off invalid input. A user could type in "fubar" and you'll miss it.

Problem 2:

Covered by partially by Robin Johnson's answer. hex_pref isn't long enough to hold "0x" and the input user argument. Fortunately it's not necessary. strtol's third argument specifies the input is in hex.

Solution: remove strcat and hex_pref.

Problem 3:

You are outputting unsigned characters. These will be interpreted by << as the ASCII value of whatever number you are fed in. This will give you nasty crap character outputs, beeps and whatever else happens to be represented by that number, not nice, clean hex values.

Solution: unsigned int key or cast key[i] to unsigned int when printing.

Semi-problem 4:

You are not outputting the results in hex.

Solution: use std::hex

Answer 3

A possible version of your code which may work is below. I would consider to think about a better design anyway. Pretty sure plenty possible issues may occur with below code as well.

unsigned long long key[16] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };


int main(int argc, char** argv) {



for (int i = 1; i < argc; i++) {

    sscanf(argv[i], "%x", &(key[i]));

    std::cout << argv[i] << "\t";
}
std::cout << std::endl << std::endl;
for (int i = 0; i < argc; i++)
    std::cout << key[i] << "\t";
std::cout << std::endl;

getchar();
return 0;

}