I'm having a bit of a pogram with making a starcraft cdkey decode function in C++. Since I'm still learning C++, I decided to create the starcraft cdkey decode function, but not by copying the version of it already made in C++ (the one made by YobGuls, I think?). I decided to look at my Python version of the function, and translate it directly to C++. However, I ended up needing to do a lot of casting, in order to make the variables interact with eachother. When I finished, I had the following code:
[code]unsigned char *DecodeStarKey(unsigned char *key) {
    unsigned char arrayKey[13];
    unsigned int i, v = 3, v2 = 0x13AC9741, n, n2, c2;
    unsigned char c;
    for(i = 1; i <= 13; i++) {
        arrayKey[i-1] = key[i-1];
    }
    for(i = 0; i <= 12; i++) {
        c = arrayKey[i];
        n = (unsigned int)(c);
        n2 = v*2;
        n ^= n2;
        v += n;
    }
    v = 194;
    for(i = 11; i >= 0; i--) {
        if (v<7)
          break;
        c = arrayKey[i];
        n = v/12;
        n2 = v%12;
        v -= 17;
        c2 = (unsigned int)arrayKey[n2];
        arrayKey[i] = (unsigned char)c2;
        arrayKey[n2] = c;
    }
    for(i = 11; i >= 0; i--) {
        c = arrayKey[i];
        arrayKey[i] = c;
        if(toupper(c) <= 55) {
            v = v2;
            c2 = v & 0xFF;
            c2 &= 7;
            c2 ^= (unsigned int)(c);
            v >>= 3;
            arrayKey[i] = (unsigned char)c2;
            v2 = v;
        } else if (toupper(c) < 65) {
            c2 = i;
            c2 &= 1;
            c2 ^= (unsigned int)(c);
            arrayKey[i] = (unsigned char)c2;
        }
    }
    for(i = 1; i <= 13; i++) {
        arrayKey[i-1] = arrayKey[(unsigned char)i-1];
    }
    return arrayKey;
}[/code]
However, whenever I try to use this function, I receive an error (in a message dialog form) about how the memory could not be "read". I have reason to believe that the return statement is causing this problem. Does anyone else have any ideas?

Note: I havn't learned much about DWORD's (and the other data types that the other cdkey decode function used that I havn't) yet. So of course I couldn't use them.

[quote author=Yegg link=topic=11921.msg116845#msg116845 date=1119452726]
However, whenever I try to use this function, I receive an error (in a

message dialog form) about how the memory could not be "read".
[/quote]

You are returning a local variable allocated on the functions stack:-

[code]
return arrayKey;
[/code]

Once the function exits, the pointer to the memory allocated on the stack for that function is no longer valid. Thats one problem i see atleast, there may be others.

To fix it you have to either dynamically allocate the array in the function like this:-

[code]
unsigned char* arrayKey = new char[13];
[/code]

Which will mean the caller will have to delete it. Proberly not the best solution.

Or the caller passes a pointer to an array which you can then fill in like this:-

[code]

// Caller
unsigned char key[] = "243538445859";
unsigned char buf[13];

// buf is used as an out parameter which the callee fills in
DecodeStarKey(key, buf);

// Callee
unsigned char *DecodeStarKey(unsigned char *key, unsigned char *outBuf)
[/code]

I've used cout to output certain data at different areas throughout the function, it all seems fine. This is why "I have reason to believe" that the return statement causes the problem. How should I correctly arrange that statement then?

Update: Thanks for the help, I'll try it out now. I didn't notice you edited your post.

Your directions wern't clear enough to me, but here is my new code:
[code]unsigned char *DecodeStarKey(unsigned char *key, unsigned char *outBuf) {
    unsigned char arrayKey[13];
    unsigned int i, v = 3, v2 = 0x13AC9741, n, n2, c2;
    unsigned char c;
    for(i = 1; i <= 13; i++) {
        arrayKey[i-1] = key[i-1];
    }
    for(i = 0; i <= 12; i++) {
        c = arrayKey[i];
        n = (unsigned int)(c);
        n2 = v*2;
        n ^= n2;
        v += n;
    }
    v = 194;
    for(i = 11; i >= 0; i--) {
        if (v<7)
          break;
        c = arrayKey[i];
        n = v/12;
        n2 = v%12;
        v -= 17;
        c2 = (unsigned int)arrayKey[n2];
        arrayKey[i] = (unsigned char)c2;
        arrayKey[n2] = c;
    }
    for(i = 11; i >= 0; i--) {
        c = arrayKey[i];
        arrayKey[i] = c;
        if(toupper(c) <= 55) {
            v = v2;
            c2 = v & 0xFF;
            c2 &= 7;
            c2 ^= (unsigned int)(c);
            v >>= 3;
            arrayKey[i] = (unsigned char)c2;
            v2 = v;
        } else if (toupper(c) < 65) {
            c2 = i;
            c2 &= 1;
            c2 ^= (unsigned int)(c);
            arrayKey[i] = (unsigned char)c2;
        }
    }
std::cout << "1\n";
    for(i = 1; i <= 13; i++) {
        arrayKey[i-1] = arrayKey[(unsigned char)i-1];
    }
return outBuf;
}/code]
buf does get declared under main() and when the function is called it includes the key, and the buf variable. No warnings are issued, but I still receive the same error. "The memory could not be read".

Update: I think I now know what I have to do. Making a pointer from arrayKey to *key might be the solution. Let me try this.[/code]

This is what OnlyMeat means by "You are returning a variable allocated on the local function's stack" :

When you return a pointer or array, it should be done via an out pointer or reference, not via a return value.  You can generate a memory leak by not de-allocating the pointers declared in your DecodeStarKey function.

A couple other stylistic notes (OnlyMeat, feel free to tell me I don't know what I'm talking about in a non-GC'd language):

You said you're using C++.  Instead of just declaring arrays, use the new operator:
[code]
unsigned char arrayKey[] = new unsigned char[13];
[/code]

Now that you've declared the array, be sure to delete it before exiting the function:
[code]
delete[] arrayKey;
[/code]
Remember to delete your pointers when they're out of scope.

You don't need to return outBuf.  You can simply mark the function as void and return nothing; as long as you've been assigning values to the array pointed to by outBuf, you're okay (which you're not doing in the latest code you posted).  You can also follow the typical function prototypes used by Microsoft and return a BOOL value, indicating success or failure of the function.  Return 0 if it succeeded, or a nonzero value indicating an error.

Finally, I'd suggest passing an int value that tells the DecodeStarKey function how long the outBuf buffer is.  You don't *have* to, but if you don't, and you accidentally pass in 3-byte buffer instead of a 13-byte buffer, you'll be overwriting other memory, which could overwrite other values, generating runtime errors that you can't quite pinpoint.

[quote author=Yegg link=topic=11921.msg116889#msg116889 date=1119464553]
Your directions wern't clear enough to me, but here is my new code:
[/quote]

Try this for the decode function:-

[code]
void DecodeStarKey(const unsigned char *key, unsigned char *outBuf)
{
    unsigned int i, v = 3, v2 = 0x13AC9741, n, n2, c2;
    unsigned char c;
    for(i = 1; i <= 13; i++) {
        outBuf[i-1] = key[i-1];
    }
    for(i = 0; i <= 12; i++) {
        c = outBuf[i];
        n = (unsigned int)(c);
        n2 = v*2;
        n ^= n2;
        v += n;
    }
    v = 194;
    for(i = 11; i >= 0; i--) {
        if (v<7)
          break;
        c = outBuf[i];
        n = v/12;
        n2 = v%12;
        v -= 17;
        c2 = (unsigned int)outBuf[n2];
        outBuf[i] = (unsigned char)c2;
        outBuf[n2] = c;
    }
    for(i = 11; i >= 0; i--) {
        c = outBuf[i]; // what are these two lines doing?
        outBuf[i] = c; // ...
        if(toupper(c) <= 55) {
            v = v2;
            c2 = v & 0xFF;
            c2 &= 7;
            c2 ^= (unsigned int)(c);
            v >>= 3;
            outBuf[i] = (unsigned char)c2;
            v2 = v;
        } else if (toupper(c) < 65) {
            c2 = i;
            c2 &= 1;
            c2 ^= (unsigned int)(c);
            outBuf[i] = (unsigned char)c2;
        }
    }
std::cout << "1\n";
    for(i = 1; i <= 13; i++) {
        outBuf[i-1] = outBuf[(unsigned char)i-1];
    }
        // You no longer need to return a value
//return outBuf;
}
[/code]

Then when you call the decode function you can do this:-
[code]
unsigned char key[] = "blahblahblah...";
unsigned char buf[13];
DecodeStarKey(key, buf);
[/code]

buf should then contain the decoded information you require, provided your function works of course.

Looks like you both are forgetting to check if the cdkey is valid also.

I must suck at life or somethin. This time it says "The memory could not be "written"". Here is my entire program:
[code]#include <iostream>
#include <stdio.h>
void DecodeStarKey(const unsigned char *key, unsigned char *outBuf);

int main(void) {
unsigned char user_input[13];
unsigned char buf[13];
std::cout << "Enter a starcraft cdkey: ";
std::cin >> user_input;
std::cout << user_input << "\n";
std::cout << "Starting decoding process...\n";
DecodeStarKey(user_input, buf);
    return 0;
}

void DecodeStarKey(const unsigned char *key, unsigned char *outBuf)
{
    unsigned int i, v = 3, v2 = 0x13AC9741, n, n2, c2;
    unsigned char c;
    for(i = 1; i <= 13; i++) {
        outBuf[i-1] = key[i-1];
    }
    for(i = 0; i <= 12; i++) {
        c = outBuf[i];
        n = (unsigned int)(c);
        n2 = v*2;
        n ^= n2;
        v += n;
    }
    v = 194;
    for(i = 11; i >= 0; i--) {
        if (v<7)
          break;
        c = outBuf[i];
        n = v/12;
        n2 = v%12;
        v -= 17;
        c2 = (unsigned int)outBuf[n2];
        outBuf[i] = (unsigned char)c2;
        outBuf[n2] = c;
    }
    for(i = 11; i >= 0; i--) {
        c = outBuf[i]; // what are these two lines doing?
        outBuf[i] = c; // ...
        if(toupper(c) <= 55) {
            v = v2;
            c2 = v & 0xFF;
            c2 &= 7;
            c2 ^= (unsigned int)(c);
            v >>= 3;
            outBuf[i] = (unsigned char)c2;
            v2 = v;
        } else if (toupper(c) < 65) {
            c2 = i;
            c2 &= 1;
            c2 ^= (unsigned int)(c);
            outBuf[i] = (unsigned char)c2;
        }
    }
    for(i = 1; i <= 13; i++) {
        outBuf[i-1] = outBuf[(unsigned char)i-1];
    }
    // You no longer need to return a value
//return outBuf;
}[/code]
Once again, no warnings are issued. I still have no clue what could be causing the error.
PS. Thanks for all the help so far.

Learn how to debug.  Step through and tell us which line causes the error.

Check this out:
[code]
unsigned int i;

for(i = 11; i >= 0; i--) {
[/code]
when i = 1 the code in the for loop is executed.
when i = 0 the code in the for loop is executed.
now i, since it's unsigned, underflows to 4294967295, so the code in the for loop is executed.

and your code uses i as an index to the array of 13 elements. 

Solution: make i a signed integer.

Sorry for not responding sooner. I changed all unsigned variables to signed and I fixed a few other minor problems and the function works fine, it sends back correct data as well. Thanks for all the help you guys.

Post the working code of the function? Who knows my come in use later, i know im just starting C++ and i'll eventually need the decode  ;D

[code]signed char *DecodeStarKey(const signed char *key, signed char *outBuf)
{
    signed int i, v = 3, v2 = 0x13AC9741, n, n2, c2;
    signed char c;
    for(i = 1; i <= 13; i++) {
        outBuf[i-1] = key[i-1];
    }
    for(i = 0; i <= 12; i++) {
        c = outBuf[i];
        n = (signed int)(c);
        n2 = v*2;
        n ^= n2;
        v += n;
    }
    v = 194;
    for(i = 11; i >= 0; i--) {
        if (v<7)
          break;
        c = outBuf[i];
        n = v/12;
        n2 = v%12;
        v -= 17;
        c2 = (signed int)outBuf[n2];
        outBuf[i] = (signed char)c2;
        outBuf[n2] = c;
    }
    for(i = 11; i >= 0; i--) {
        c = outBuf[i];
        if(toupper(c) <= 55) {
            v = v2;
            c2 = v & 0xFF;
            c2 &= 7;
            c2 ^= (signed int)(c);
            v >>= 3;
            outBuf[i] = (signed char)c2;
            v2 = v;
        } else if (toupper(c) < 65) {
            c2 = i;
            c2 &= 1;
            c2 ^= (signed int)(c);
            outBuf[i] = (signed char)c2;
        }
    }
    for(i = 1; i <= 13; i++) {
        outBuf[i-1] = outBuf[(signed char)i-1];
    }
return outBuf;
}[/code]
This will return the decoded key plus the original key, with some identical symbols between.