We help IT Professionals succeed at work.

Memory leak in function

Dimkov
Dimkov asked
on
579 Views
Last Modified: 2013-12-14
I have memory leak here, but I can't find it:

char* getFile (wstring str,unsigned int &size)
{
      FILE * pFile;
      long lSize;
      char * buffer1;
      size_t result;

      pFile = _wfopen ( str.c_str() , L"rb" );


      if (!pFile) return NULL;
      // obtain file size:
      fseek (pFile , 0 , SEEK_END);
      lSize = ftell (pFile);
      rewind (pFile);
      char bom[3] = { 0xef, 0xbb, 0xbf };
      fread(bom, 1, 3, pFile);
      bool dali=true;
      if (bom[0]!=-17 && bom[1]!=-69 && bom[2]!=-65)
      {
            dali=false;
            rewind(pFile);
      }

      // allocate memory to contain the whole file:
      buffer1 = (char*) malloc (sizeof(char)*lSize);
      // copy the file into the buffer:
      result = fread (buffer1,1,lSize,pFile);
      if (!dali)
      {
            buffer1[lSize]='\0';
            size=sizeof(char)*(lSize+1);
      }
      else
      {
            buffer1[lSize-3]='\0';
            size=sizeof(char)*(lSize-2);
      }

      fclose (pFile);
      return buffer1;
}      

Result CeVemSetcce::AddSignature(SIG_TYPE type, CertInfo cert)
{
     HRESULT hr;
      //create the component
    CComPtr<a> a;
    hr = a.CoCreateInstance(L"a.1");

      //create the prop structure
      CComPtr<b> b;
    hr = a->CreatePropObject(&b);
      
      //create the certificate
    CComPtr<c> c;
      a->CreateCertInfoObject(&h);
      hr = spCertHash->put_certHashB64(ToBstr(cert.certHash));

      if (type==MY_TYPE)
      {
            char *xml;
            unsigned int size;

            xml=getFile(XMLDoc,size);
            if (xml==NULL)
            {
                  Result r;
                  r.error=0;
                  r.errorMsg="The file could not be found";
                  return r;
            }
            wchar_t *dest=(wchar_t*) malloc(size*sizeof(wchar_t));
            MultiByteToWideChar(CP_UTF8, 0, xml, -1, dest, size);
            wstring str(dest);
            if (str.find(L"ds:Something")!=wstring.npos)
            {
                  Result r;
                  r.error = 0;
                  r.errorMsg = "error";
                  free(dest);
                  free(xml);
                  return r;
            }
            
            int position =0;
            string path="0;X;1;0;X;";
            int brojac=0;
            while ((position=str.find(L"<doc:Attachment ",position+14))!=wstring::npos && str.length()>0)
            {
                  char st[20];
                  string newAttachment=string("2;")+itoa ( brojac, st, 10)+ string(";X;");
                  path.append(newAttachment);
                  brojac++;
            }
            path.append("@;");
            
            free(dest);
            dest=NULL;
            free(xml);
            xml=NULL;
      }
      else if (type==MYTYEP2)
      {
            
            char *xml;
            unsigned int size;

            xml=getFile(XMLDoc,size);
            if (xml==NULL)
            {
                  Result r;
                  r.error=0;
                  r.errorMsg="The file could not be found";
                  return r;
            }
            wchar_t *dest=(wchar_t*) malloc(size*sizeof(wchar_t));
            MultiByteToWideChar(CP_UTF8, 0, xml, -1, dest, size);
            wstring str(dest);
            if (str.find(L"ds:something")!=string::npos)
                       int d=6;
            else
            {
                  Result r;
                  r.error = 0;
                  r.errorMsg = "errpr";
                  free(dest);
                  free(xml);
                  return r;
                  
            }
            free(dest);
            free(xml);
      }
    CComPtr<r> r;
    hr = a->Sign(b, &r);
      
        
      short vaError;
      hr = r->get_error(&vaError);

      CComBSTR errMsg;
    hr = r->get_errorMessage(&errMsg);

      CComBSTR str;
      hr = r->get_xml(&str);
      
      long size;
      hr = r->get_xmlLen(&size);

        Result r;

      USES_CONVERSION;
      r.cInfo.error =certError;
      r.cInfo.issuer=W2CA(vaIssuer);
      r.cInfo.serialNumber=W2CA(vaSN);
      r.cInfo.status[0]=vaStatus[0];
      r.cInfo.subject=W2CA(vaSubject);
      r.error = vaError;
      r.errorMsg = W2CA(errMsg);
      r.sXml = str;
      return r;
}

the com component is fine, so the error is in one of these lines.
since in the dump i find "<?xml version" i believe it is  the str variable... but i cant see any leakage
Comment
Watch Question

Commented:
Check your malloc()s, you may be allocating one byte too few, as you need space for a terminating zero byte quite often.

Author

Commented:
how can i check them?

{349} normal block at 0x003DFF68, 3 bytes long.
 Data: <   > 00 CD CD
{337} normal block at 0x01C62C80, 48218 bytes long.
 Data: <<?xml version="1> 3C 3F 78 6D 6C 20 76 65 72 73 69 6F 6E 3D 22 31

Commented:
You might want to provide your own wrapper functions around your malloc/free calls - the former wrapper can also be helpful for handling any case where malloc fails - which you don't appear to be checking?

If you're using reasonable up-to-date headers [and compiler] you should also removes the malloc return casts.

>>>> W2CA(vaIssuer);
I assume it is here as the W2CA conversion most likely creates a buffer which need to be freed.

Note, they most likely use 'new' operator (what you should done as well in a C++ prog!), so you have to 'delete' the buffers.

Regards, Alex

Commented:
Comment1:

buffer1 = (char*) malloc (sizeof(char)*lSize);

So, you allocated from 0 to lSize-1 items

buffer1[lSize]='\0';

Hmm, off the end I think. You should alloc one more spot if you do this.

buffer1 = (char*) malloc (sizeof(char)*(lSize+1));

I see that you do not free buffer1 in the same routine that allocates it.

This appears to be C++, yet you use malloc rather than new. Why? I was told to never mix memory models.

If you believe that you know which variable has the leak, choose a point when you are finished with the variable and add known text to it. For example, set the str to something like "121212". Just before you free some of your buffers, set them to other easily identified values.


>>>       char bom[3] = { 0xef, 0xbb, 0xbf };
>>>         fread(bom, 1, 3, pFile);
>>>         bool dali=true;
>>>         if (bom[0]!=-17 && bom[1]!=-69 && bom[2]!=-65)
That is no good code. It makes less sense to read data into a a buffer initialized with the char's to compare. Moreover, in the 'if' you have to use 'or' to find out whether it is not a bom.

      char bom[3] = { 0xef, 0xbb, 0xbf };
      char filebegin[3];
      fread(filebegin, 1, 3, pFile);
      bool dali=true;
      if (bom[0] != filebegin[0] || bom[1]!= filebegin[1] || bom[2]!=filebegin[2])
         ...
 

Author

Commented:
I use W2CA  with many variables, but only "<?xml version=.. "seems to be wrong...
I am guessing it is in the getFile function but I am not sure

Author

Commented:
for it to be BOM, it needs to have ALL 3 character or ANY of those characters?.
Thanks for the other tip. I will change it :)
bijopuliSoftware Architect

Commented:
Hi

Instead of using malloc and free, try using new and delete.


Regards
Bijo.
Unlock this solution and get a sample of our free trial.
(No credit card required)
UNLOCK SOLUTION
>>>> I am guessing it is in the getFile function but I am not sure
Hmmm. the only allocation in getfile is for buffer1 which was assigned to xml in any case. All exit branches were freeing the xml.

>>>> wstring getxml(const string& str);
should be
   wstring getxml(const string& str) {
>>>>I use W2CA  with many variables,
You are right. W2CA uses a buffer on the stack up to 500 KB.

>>>> but only "<?xml version=.. "seems to be wrong...
The text in the leak buffer is ANSI not UNICODE. So it indeed looks like it is the BOM (3 bytes) and the binary stream of an of the ANSI xml file (48218  bytes). Can you verify if that is the file size of the xml? Nevertheless, the code above doesn't fit to the leak. You neither allocate storage for the BOM at the heap nor is there a way that the buffer1 was not freed. If you are sure that the above code is producing the leak (it also can be produced when writing the xml and - say the file wasn't closed), I could only think that some of the code is corrupted what prevents from actually beeing freed. But that should be the last resort to look for.

You might consider using the DEBUG_NEW macro to locate the leaks. Put the following below your header files and try compiling.

#ifdef _DEBUG
#define new DEBUG_NEW
#endif

If you got errors you temporarily need to include #include <afxwin.h> as very first include (if using precompiled headers you have to put it into you stdafx.h and make a rebuild).

If using DEBUG_NEW you'll get additional information for most leaks: the file and the line where the allocation happened.



Author

Commented:
thanks Alex, that solved the problem
AxterSenior Software Engineer
CERTIFIED EXPERT

Commented:
I recommend using the following library:
http://code.axter.com/leaktracker.h
Unlock the solution to this question.
Thanks for using Experts Exchange.

Please provide your email to receive a sample view!

*This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.

OR

Please enter a first name

Please enter a last name

8+ characters (letters, numbers, and a symbol)

By clicking, you agree to the Terms of Use and Privacy Policy.