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

Custom checksum algorithm

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

Problem

A while back, I reverse-engineered a checksum algorithm from an MMO used to check the validity of an item that's linked to chat (similar to WoW). The idea is that if the checksum is invalid then the game client would ignore the link when clicked. Otherwise clicking on the item link in in-game chat would display the stat and attrib for that item.

ushort16 CreateChecksum(const string &itemlink)
{
    stringstream parseitemlink(itemlink);
    uint32 hexform[ITEMLINKGROUPCOUNT] = {0};
    uint32 hexsum = 0;

    //Parse itemLink string into hexform array
    for (int i = 0; i > hex >> hexform[i];

    //sum all the itemlink group together
    for (int i = 0; i > 16; // aka same as dividing hexform[i] by 65,536

        // (hexform[i] / 65,536) + (hexform[i] * i) + hexsum -- for odd groups
        //  hexform[i] + (hexform[i] * i) + hexsum           -- for even groups
        ebx += edi + hexsum;
        hexsum = ebx ^ hexform[i];
    }

    for (int i = 0; i >= 16;
        eax += ecx;
        hexsum = eax ^ hexform[i];
    }

    //return the lower 2-bytes of hexsum
    //as the final checksum
    return hexsum & 0xFFFF;
}//CreateChecksum


The format of the itemlink is comprised of a group of hexadecimal separated with a space in string format. It's passed into main() as an argument when the program is run.

Here's what an itemlink's hex string might look like:

const string EXAMPLELINK = "36c6a 0 3f000a54 d0f1 0 0 0 0 0 0 20d0";


Are there any code smells or readability issues in this code segment? Can any part(s) of it be improved?

Solution

If you are using the standard library classes of the same name, I would give the following names the correct namespace qualifier: std::string, std::stringstream, std::hex.

In C++, this works just as well, IMHO it's mildy more idiomatic.

uint32 hexform[ITEMLINKGROUPCOUNT] = {};


ebx, edi, ecx, eax are not good variable names, if you can give them more meaningful names, then do.

uint32 ecx = (hexform[i] + 1) * hexsum,
           eax = ecx * hexform[i];


Personally, I think this is clearer:

uint32 ecx = (hexform[i] + 1) * hexsum;
    uint32 eax = ecx * hexform[i];


The comment is really bad because it talks about hexform[i]^2 + hexform[i] hexsum whereas ecx gets the value hexform[i] hexsum + hexsum and eax gets the value hexform[i]^2 hexsum + hexform[i] hexsum. I think the comment needs a pair of parentheses if the code is doing what you meant.

To be robust, you should check whether the parse worked.

parseitemlink >> hex >> hexform[i];


You can trivially combine the first two for loops as well.

Code Snippets

uint32 hexform[ITEMLINKGROUPCOUNT] = {};
uint32 ecx = (hexform[i] + 1) * hexsum,
           eax = ecx * hexform[i];
uint32 ecx = (hexform[i] + 1) * hexsum;
    uint32 eax = ecx * hexform[i];
parseitemlink >> hex >> hexform[i];

Context

StackExchange Code Review Q#22, answer score: 10

Revisions (0)

No revisions yet.