• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 559
  • Last Modified:

Memory leak in function

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
0
Dimkov
Asked:
Dimkov
1 Solution
 
grg99Commented:
Check your malloc()s, you may be allocating one byte too few, as you need space for a terminating zero byte quite often.

0
 
DimkovAuthor 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
0
 
peetmCommented:
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.

0
Concerto's Cloud Advisory Services

Want to avoid the missteps to gaining all the benefits of the cloud? Learn more about the different assessment options from our Cloud Advisory team.

 
itsmeandnobodyelseCommented:
>>>> 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
0
 
pitonyakCommented:
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.


0
 
itsmeandnobodyelseCommented:
>>>       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])
         ...
 
0
 
DimkovAuthor 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
0
 
DimkovAuthor 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 :)
0
 
bijopuliCommented:
Hi

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


Regards
Bijo.
0
 
itsmeandnobodyelseCommented:
>>>> Hmm, off the end I think. You should alloc one more spot if you do this.

pitonyak makes a good point. You could get rid of all these problems (here and in the previous thread) by writing and reading using the fputws and fgetws functions (they would write/read an ANSI xml) and returning a wstring instead of a wchar_t buffer.

wstring getxml(const string& str);
       FILE * pf = fopen(str.c_str(), "rt,css=UTF-8 ");
       wstring ws;
       wchar_t wsz[8192];  // big enough for very long lines
       while (fgetws(wsz, sizeof(wsz), pf) != NULL)
       {
               ws += wsz;
       }
        fclose(fp);
        return ws;
}

Regards, Alex
0
 
itsmeandnobodyelseCommented:
>>>> 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) {
0
 
itsmeandnobodyelseCommented:
>>>>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.



0
 
DimkovAuthor Commented:
thanks Alex, that solved the problem
0
 
AxterCommented:
I recommend using the following library:
http://code.axter.com/leaktracker.h
0

Featured Post

Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Tackle projects and never again get stuck behind a technical roadblock.
Join Now