CodeGuard gefällt "delete [] Buffer" nicht

wedmer

Hallo,
um eine Datei zu schreiben benötige ich TFileStream. Um die Datei zuschreiben arbeite ich mit einem Buffer, den habe ich folgender massen definiert:

char* Buffer = new char[64];

wenn ich alle gewünschten Daten in der Datei versorgt habe, möchte ich den Buffer folgendermassen wieder freigeben:

delete [] Buffer;

doch nun kommt CodeGuard und meint:

Bad parameter
|
|- A bad object array (0x4B4332) has been passed to the funciton
|-__org_deletea
  |
  |-TForm1::SaveNetDrv()
  |-TForm1::SaveButtonClick(const System::TObject*)
  |-0x00486A16
  |-0x00489754
  |-Controls::_16864 + 0x28
  |-0x00489754

was soll das?

Stehen die Reservierung und die Freigabe auch in einem Gültigkeitsbereich?? Meine damit, dass das delete auch noch zu einem Zeitpunkt erfolgt, wo das Array noch gültig ist.

Schon mal über

SysGetMem reserviert eine bestimmte Anzahl Bytes und gibt einen Zeiger auf den Speicherbereich zurück.

SysFreeMem gibt einen bestimmten Speicherbereich frei.

SysReallocMem gibt einen Zeiger auf eine bestimmte Anzahl von Bytes unter Beibehaltung der von Pointer angegebenen Werte zurück.

nachgedacht

wedmer

Jup, die stehen im selben Gültigkeitsbereich. Unmittelbar nach dem ich den letzten Wert ins File geschrieben habe, versuche ich den Buffer wieder frei zu geben.
Ich denke für diese Aufgabe sollte doch new und delete ausreichen.

PuppetMaster2k

Zeig doch mal den kompletten Code-Teil, damit wir uns ein besseres Bild von dem Problem machen können.

wedmer

Ok, hier ist der gesamt Code:

//---------------------------------------------------------------------------
// saves the net drives
//---------------------------------------------------------------------------
void Main::SaveNetDrv()
{
   AnsiString Version = "1.1";
   AnsiString FileName =  ExtractFilePath(Application->ExeName)+
                          "lans.wdi";
   char BufferSize = 64;
   char* Buffer = new char[BufferSize];
   int Int;
   int* pInt;
   pInt = &Int;
   int StringSize;
   TFileStream* File;
   int LANCount;
   int ServerCount;
   int NetDriveCount;

 //open file to overwrite
   if(FileExists(FileName))                            //delete file, if it exists
      DeleteFile(FileName);
   else;
   File = new TFileStream(FileName,fmCreate);          //create file

 //write header (application name with version)
   StrCopy(Buffer,"--::WediNetConnector::--[v:");
   File->Write(Buffer,27);
   StringSize = Version.Length();
   File->WriteBuffer(&StringSize,sizeof(StringSize));
   File->Write(Version.c_str(),StringSize);
   StrCopy(Buffer,"]");
   File->Write(Buffer,1);

 //write LANs
   StrCopy(Buffer,"-{LANs:");                          //write begin of LANs
   File->WriteBuffer(Buffer,7);
   LANCount = LANs.Length;                             //get number of LANs
   File->WriteBuffer(&LANCount,sizeof(LANCount));      //write number of LANs
   for(int i=0;i<LANCount;i++)                         //write LANs
   {
      *pInt = LANs[i]->LANID;                          //write LAN ID
      File->WriteBuffer(pInt,sizeof(Int));
      StringSize = LANs[i]->NameEdit->Text.Length();   //write LAN name
      File->WriteBuffer(&StringSize,sizeof(StringSize));
      File->WriteBuffer(LANs[i]->NameEdit->Text.c_str(),
                        StringSize);
      *pInt = LANs[i]->GatewayEdit->Items->Count;
      File->WriteBuffer(pInt,sizeof(Int));
      for(int j=0;j<LANs[i]->GatewayEdit->Items->Count;j++)  //write gateway
      {
         StringSize = LANs[i]->GatewayEdit->Items->
                                       Strings[j].Length();
         File->WriteBuffer(&StringSize,sizeof(StringSize));
         File->WriteBuffer(LANs[i]->GatewayEdit->Items->
                           Strings[j].c_str(),StringSize);
      }
   }
   StrCopy(Buffer,":LANs}-");                           //write end of LANs
   File->WriteBuffer(Buffer,7);

  //write Servers
   StrCopy(Buffer,"-{Servers:");                        //write begin of Servers
   File->WriteBuffer(Buffer,10);
   ServerCount = 0;                                     //write number of Servers
   for(int i=0;i<LANs.Length;i++)
      ServerCount += LANs[i]->Servers.Length;
   File->WriteBuffer(&ServerCount,sizeof(ServerCount));
   for(int i=0;i<LANs.Length;i++)
   {
      for(int j=0;j<LANs[i]->Servers.Length;j++)
      {
         *pInt = i;                                     //write LAN ID
         File->WriteBuffer(pInt,sizeof(Int));
         *pInt = j;                                     //write Server ID
         File->WriteBuffer(pInt,sizeof(Int));
         StringSize = LANs[i]->Servers[j]->NameEdit->   //write name
                      Text.Length();
         File->WriteBuffer(&StringSize,sizeof(StringSize));
         File->WriteBuffer(LANs[i]->Servers[j]->
                   NameEdit->Text.c_str(),StringSize);
         StringSize = LANs[i]->Servers[j]->UserEdit->   //write username
                      Text.Length();
         File->WriteBuffer(&StringSize,sizeof(StringSize));
         File->WriteBuffer(LANs[i]->Servers[j]->
                 UserEdit->Text.c_str(),StringSize);
         StringSize = LANs[i]->Servers[j]->             //write password
                                   Password.Length();
         File->WriteBuffer(&StringSize,sizeof(StringSize));
         File->WriteBuffer(LANs[i]->Servers[j]->
                       Password.c_str(),StringSize);
      }
   }
   StrCopy(Buffer,":Servers}-");
   File->WriteBuffer(Buffer,10);                        //write end of Servers

 //write net drives
   StrCopy(Buffer,"-{NetDrives:");                      //write begin of net drives
   File->WriteBuffer(Buffer,12);
   NetDriveCount = NetDrives.Length;
   File->WriteBuffer(&NetDriveCount,                    //write number of net drives
                     sizeof(NetDriveCount));
   for(int i=0;i<NetDriveCount;i++)
   {
      *pInt = NetDrives[i]->LANID;                      //write LAN ID
      File->WriteBuffer(pInt,sizeof(Int));
      *pInt = NetDrives[i]->ServerID;                   //write Server ID
      File->WriteBuffer(pInt,sizeof(Int));
      *pInt = NetDrives[i]->NetDriveID;                 //write net drive ID
      File->WriteBuffer(pInt,sizeof(Int));
      StringSize = NetDrives[i]->NetDrive.Length();     //write net drive name
      File->WriteBuffer(&StringSize,sizeof(StringSize));
      File->WriteBuffer(NetDrives[i]->NetDrive.c_str(),
                        StringSize);
      StringSize = NetDrives[i]->LocalDrive.Length();   //write local drive
      File->WriteBuffer(&StringSize,sizeof(StringSize));
      File->WriteBuffer(NetDrives[i]->LocalDrive.c_str(),
                        StringSize);
      *Buffer = NetDrives[i]->Connect ? '1':'0';             //write allowed to connect
      File->WriteBuffer(Buffer,1);

   }
   StrCopy(Buffer,":NetDrives}-");                           //write end of net drives
   File->WriteBuffer(Buffer,12);
   delete File;
   File = NULL;
   delete [] Buffer;
   Buffer = NULL;
}

Ich bin langsam der Meinung, dass Codeguard manchmal etwas überempfindlich ist. Hatte letztens auch eine Schleife mit TIniFile wo CodeGuard was zu meckern hatte. Kaum hab ich die Schelfe ausgeschrieben hatte CodeGuard nichts mehr auszusetzen...

Muss BufferSize vom Typ char sein? Ich dachte immer, das sollte int sein... Weiß aber nicht ob das stimmt, hat mich nur gewundert.

PuppetMaster2k

Die Funktion ist in meinen Augen eine einzige Katastrophe.

- Sämtliche mit "// write begin ..." kommentierten Zeilen kannst du auch als Konstanten anlegen, dann hast du deren Initialisierung nur ein einziges Mal in der Initialisierungliste des Kontruktors.
- BufferSize sollte vom Typ int sein.
- Da du die Versionsnummer sowieso fest im Code angibst, warum nicht gleich "--::WediNetConnector::--[v:1.1]"? Dann sparst du dir am Anfang direkt ein paar Zeilen unnötigen Code.
- Worfür pInt? Weg damit.
- Warum unbedingt char* bzw. char[]? Du nutzt an anderen Stellen auch AnsiString::c_str(), also kannst du das generell in der Funktion nutzen. Das spart dann auch noch jede Menge StrCopy-Aufrufe und macht den Code wieder etwas schöner, lesbarer, wartbar etc.

Um jetzt nur mal ein Beispeil zu geben, wie man es machen könnte:

// Header steht im private-Bereich von Main.h als const AnsiString Header
Main::Main(...)
  :Header("--::WediNetConnector::--[v:1.1]") 
{

}

void Main::SaveNetDrv()
{
   AnsiString FileName =  ExtractFilePath(Application->ExeName)+
                          "lans.wdi";
   char BufferSize = 64;
   int Int;
   int StringSize;
   TFileStream* File;
   int LANCount;
   int ServerCount;
   int NetDriveCount;

 //open file to overwrite
   if(FileExists(FileName))                            //delete file, if it exists
      DeleteFile(FileName);
   else;
   File = new TFileStream(FileName,fmCreate);          //create file

 //write header (application name with version)
   File->Write(Header.c_str(), Header.Length()); // Man siehe, eine Zeile wo vorher 7 Zeilen standen.

... // usw.

wedmer

Jo okay, mit den Konstanten hast recht, werd ich noch änderen.
Weshalb ich Buffersize vom Typ char genommen habe kann ich dir auch nicht sagen, mir ist eigentlich schon klar, dass das int sein sollte. Hab mich da irgendwie vertippt.

Kann es sein, dass CodeGuard noch einige Bugs hat? Als ich heute den Code nochmals neu kompiliert habe, hatte CodeGuard nichts mehr zu meckern und das ohne eine Änderung. Gestern habe ich auch x-mal neu kompiliert, doch da hat das nichts genützt.

Ich trau dem Ding nicht besonders weit

CodeGuard gefällt &quot;delete [] Buffer&quot; nicht

CodeGuard gefällt "delete [] Buffer" nicht