patterncppModerate
Custom checksum algorithm
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.
The format of the
Here's what an
Are there any code smells or readability issues in this code segment? Can any part(s) of it be improved?
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;
}//CreateChecksumThe 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:
In C++, this works just as well, IMHO it's mildy more idiomatic.
Personally, I think this is clearer:
The comment is really bad because it talks about
To be robust, you should check whether the parse worked.
You can trivially combine the first two for loops as well.
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.