[RESOLVED] Memory leak?

i have this simple code that will get the HDD Serial Id into a string then XOR it using a key and then HTTP Encode so i can safely send it using InternetOpenUrl() api

the problem is that on my HTTPEncode() function , the passing string gets magically Nulled or altered after the 1 line of code

any help would be appreciated

code:

Code:

---

#include <iostream>
#include <windows.h>
#include <Wininet.h>
#pragma comment( lib, "Advapi32.lib")
#pragma comment( lib, "WinInet.lib")
using namespace std;

char* GetVolumeID();
char* XOREncryption(char* szData , char* szKey);
char* HTTPEncode(const char* AStr);

char* HTTPEncode(const char* AStr)
{
        //based on zedwood.com code
    string URL = "";
        int string_gets_altered_when_it_reaches_this_point = 1;

    for(int i=0; i<strlen(AStr); i++)
    {
        if ( (48 <= AStr[i] && AStr[i] <= 57) ||//0-9
            (65 <= AStr[i] && AStr[i] <= 90) ||//abc...xyz
            (97 <= AStr[i] && AStr[i] <= 122) || //ABC...XYZ
            (AStr[i]=='~' || AStr[i]=='!' || AStr[i]=='*' ||
                          AStr[i]=='(' || AStr[i]==')' || AStr[i]=='\'')
        )
        {
                        URL.append( &AStr[i], 1);

        }
        else
        {
                        char* tmp = new char[3];
                        sprintf(tmp, "%x", AStr[i]);
                        URL.append("%");
                        URL.append(tmp);
                        delete[] tmp;
        }
    }

        char * Ret= new char[URL.length() + 1];
        strcpy(Ret, URL.c_str());

    return Ret;
}

char* XOREncryption(char* szData , char* szKey)
{

  DWORD i, x = 0;
  char* szTemp = new char [strlen(szData)];
  szTemp[strlen(szData)] = 0;

  for (int i = 0; i <= (strlen(szData)-1); i++)
  {
  szTemp[i] = szData[i]^szKey[x];
  x >= (strlen(szKey)-1) ? x=0 : x++;
  }

return szTemp;
}


char* GetVolumeID()
{
DWORD dwNull1, dwNull2, dwID;
char szSys[MAX_PATH];
GetSystemDirectoryA(szSys,MAX_PATH);
char Drive[4] = {szSys[0],szSys[1],szSys[2]};
char cSerial[100];

if (GetVolumeInformationA(Drive,0,0,&dwID, &dwNull1, &dwNull2, 0, 0))
  return ultoa(dwID,cSerial,10);
}

int main()
{
 char* ID = GetVolumeID();
 char* szID = HTTPEncode(XOREncryption(ID, "Random"));

return 0;
}

---

The char * returned by XOREncryption can contain '\0'. You cannot call strlen on that string.
Kurt

I'm not sure why, but this line:

Code:

---

char* szTemp = new char [strlen(szData)];

---

is zeroing out your string.

At any rate, why not XOR the string in-place? Much easier and doesn't leave that new()'d char * dangling.

Code:

---

char* XOREncryption(char* szData , char* szKey)
{
  size_t pwlen=strlen(szKey);
  size_t ptlen=strlen(szData);

  for(size_t i=0;i<ptlen;i++){
    szData[i]^=szKey[i&#37;pwlen];
  }
  return szData;
}

---

Quote:

---

Originally Posted by Cpp_Noob

any help would be appreciated

---

As far as memory leaks are concerned, you could rewrite this entire code so that it uses std::vector<char> and std::string, instead of char pointers, new char[] etc. Then you don't have any memory leaks since you wouldn't be calling the allocator.

In general, whenever you're writing a C++ program, the first thing that should be done is try your hardest to not introduce char* and C-string functions into the code. That could have easily been done with the code you posted.

Also, you are going overboard with the new char[] stuff. Look at this code:

Code:

---

                char* tmp = new char[3];
                sprintf(tmp, "%x", AStr[i]);
                URL.append("%");
                URL.append(tmp);
                delete[] tmp;

---

Why are you dynamically allocating tmp? Why not just declare an array of 3 characters? What you're doing with this code is that you are needlessly calling the allocator. If this code is performed in a loop, you have introduced a bottleneck in your program, since new[] and delete[] are called over and over again.

Regards,

Paul McKenzie

@hoxsiew
i used ur code but i still get the same error :(

another thing i noticed is this

Code:

---

int main()
{
 char* ID = GetVolumeID();
 char* XOR = XOREncryption(ID, "Random");
 char* szID = HTTPEncode(XOR);
 int test = 1;
return 0;
}

---

if i add breakpoints to all 4 lines , when the 3 breakpoint is triggered ID and XOR strings have the same value :confused:

As an example, here is the same code, rewritten with no calls to new.

Code:

---

#include <windows.h>
#include <Wininet.h>
#pragma comment( lib, "Advapi32.lib")
#pragma comment( lib, "WinInet.lib")
#include <iostream>
#include <string>
#include <vector>
#include <cstdio>

using namespace std;

std::string GetVolumeID();
std::string XOREncryption(const char* szData , const char* szKey);
std::string HTTPEncode(const std::string& AStr);

std::string HTTPEncode(const std::string& AStr)
{
    //based on zedwood.com code
    string URL = "";
    for(int i=0; i<AStr.size(); ++i)
    {
        if ( (48 <= AStr[i] && AStr[i] <= 57) ||//0-9
            (65 <= AStr[i] && AStr[i] <= 90) ||//abc...xyz
            (97 <= AStr[i] && AStr[i] <= 122) || //ABC...XYZ
            (AStr[i]=='~' || AStr[i]=='!' || AStr[i]=='*' ||
            AStr[i]=='(' || AStr[i]==')' || AStr[i]=='\'')
            )
        {
            URL.append( &AStr[i], 1);
        }
        else
        {
            char tmp[3];
            sprintf(tmp, "&#37;x", AStr[i]);
            URL.append("%");
            URL.append(tmp);
        }
    }
    return URL;
}

std::string XOREncryption(const char* szData , const char* szKey)
{

    DWORD x = 0;
    std::string szTemp;
    for (DWORD i = 0; i <= (strlen(szData)-1); ++i)
    {
        szTemp += static_cast<char>(szData[i]^szKey[x]);
        x >= (strlen(szKey)-1) ? x=0 : x++;
    }
    return szTemp;
}


std::string GetVolumeID()
{
    DWORD dwNull1, dwNull2, dwID;
    char szSys[MAX_PATH];
    GetSystemDirectoryA(szSys,MAX_PATH);
    char Drive[4] = {szSys[0],szSys[1],szSys[2]};
    char cSerial[100];

    if (GetVolumeInformationA(Drive,0,0,&dwID, &dwNull1, &dwNull2, 0, 0))
        return ultoa(dwID,cSerial,10);
}

int main()
{
    string ID = GetVolumeID();
    string szID = HTTPEncode(XOREncryption(ID.c_str(), "Random"));

    return 0;
}

---

There is a problem with your GetVolumeID() function. The return value is not specified if GetVolumeInformation returns false.

Regards,

Paul McKenzie

thanks Paul McKenzie , i just have a "bad" habit using char* strings on simple applications
works fine now :)

Quote:

---

Originally Posted by Cpp_Noob

works fine now :)

---

But if you are still interested in what was wrong - you have returned a pointer to local (stack) variable from your GetVolumeID() function. Next time you call a function (or use a stack in some other way) that memory might get overwritten.
Didn't your compiler warn you? C4172? "returning address of local variable or temporary"?

yes im still interested on whats wrong VladimirF
how can i prevent the variable from getting overwritten?

im using VC 2008 , no warnings about this :(

Quote:

---

Originally Posted by Cpp_Noob

how can i prevent the variable from getting overwritten?

---

You can't. Just don't (ever) return a pointer to local variable.
In your case, you could declare that

Code:

---

char cSerial[100];

---

in your main() function, and pass its pointer to GetVolumeID().
Or what Paul said.

Quote:

---

Originally Posted by VladimirF

Or what Paul said.

---

+1.

Why not leverage C++ and use string classes instead of the raw char* and string functions?

You'll end up making your code cleaner and won't spend time tracking down issues with manually manipulating the strings yourself.

Some programmers consider it to be a source of pride and feel that they HAVE to manually manipulate strings. Other programmers are in the mindset where they want to use the tools available (like string classes) and are more interested in solving the challenges of their particular program rather than spending time debugging the mundane.

It's one thing to understand raw string manipulation while learning (and it's a good thing), but IMO you'll want to get off that bus as soon as possible and get on the using string classes bus.

Quote:

---

Originally Posted by Arjay

Some programmers consider it to be a source of pride and feel that they HAVE to manually manipulate strings. Other programmers are in the mindset where they want to use the tools available (like string classes) and are more interested in solving the challenges of their particular program rather than spending time debugging the mundane.

---

There was a discussion here a while back on C++ programmers using STL and other libraries over coding their own routines. Basically some people were making the claims that by using STL, string classes, etc., you don't learn the other lower-level parts of C++ to a great extent. I think this is what a lot of this "do everything myself" attitude comes from -- if you use the C++ library, you're a "stupid" programmer who can't write low-level code.

To that I responded to that by saying "try to find such a programmer".

Every single programmer I know, and I bet most, if not all the programmers that primarily uses STL to do advanced work already know how to handle dynamically allocated memory properly and appropriately, how to write a map or linked list class if necessary, etc. You will be hard-pressed to find in the industry an STL-centric C++ programmer who doesn't know how to write low-level code. If you can find one, that is one rare individual.

Regards,

Paul McKenzie

I agree. I believe the thought is that if a programmer can code the low level things, then the programmer can learn and code anything at a higher level.

Unfortunately, often times there ends up being programmers that don't make that transition and prefer for what ever reasons to stay at the lower level.

Quote:

---

Originally Posted by Arjay

I agree. I believe the thought is that if a programmer can code the low level things, then the programmer can learn and code anything at a higher level.

---

To that, there are a lot of "low-level" programmers who's eyes glaze over when they look at STL code. They have no idea where to start or what it means. In many instances, you can forget about trying to get these programmers to write code at a higher level using algorithms, iterators, etc. It just isn't going to happen, either because they just don't understand it, or they stick with what they know, or both.

This scenario by far happens more often than the other scenario of a brilliant C++ programmer who uses STL, and cannot write low-level code. I will be daring and make the claim that you cannot find any advanced C++ programmer who uses STL and the C++ library who cannot write 'C' style low-level code.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

To that, there are a lot of "low-level" programmers who's eyes glaze over when they look at STL code. They have no idea where to start or what it means. In many instances, you can forget about trying to get these programmers to write code at a higher level using algorithms, iterators, etc. It just isn't going to happen, either because they just don't understand it, or they stick with what they know, or both.

This scenario by far happens more often than the other scenario of a brilliant C++ programmer who uses STL, and cannot write low-level code. I will be daring and make the claim that you cannot find any advanced C++ programmer who uses STL and the C++ library who cannot write 'C' style low-level code.

Regards,

Paul McKenzie

---

that is sooo true Paul McKenzie , most of my codes are "low level" and my eyes do glaze when looking over at STL code.

Well, if you are still interested in original code analysis: you did not get that warning because compiler got confused by your returning whatever ultoa() returns. Obviously, the compiler doesn’t know that ultoa() returns the same pointer you pass to it.
However, when I compiled your function (with VS 2008) I did get a warning:
warning C4715: 'GetVolumeID' : not all control paths return a value
If you didn’t see it – it means that your warning level compiler switch is NOT set to 4 (/W4) as it should.

Quote:

---

Originally Posted by Cpp_Noob

that is sooo true Paul McKenzie , most of my codes are "low level" and my eyes do glaze when looking over at STL code.

---

STL becomes a lot easier and more appealing once you take the time to play around with it a bit.