Board index » delphi » What am I doing Wrong in this Code?

What am I doing Wrong in this Code?

Ok, I'm starting to pull my hair out.  Could someone please tell me
what I'm doing wrong?  Any help would be greatly appreciated.  Its
trashing memory somehow, I just don't see where.  (When run, it causes
Win95 to drop to a command prompt with "invalid COMMAND.COM" error,
not very pleasant)

function CopyFile(Source, Dest: string; OverWriteFile, MoveFile:
boolean): integer;

const BufferSize = 32000;

type BufferType = array[1..BufferSize] of Byte;

var inFileHandle, outFileHandle: longint;
    Buffer: ^BufferType;
    BytesToRead, TotalBytes, TotalFileSize: longint;
    TempString1, TempString2: PChar;
    FileStruct1, FileStruct2: TOfStruct;
    rc: longint;
    done: boolean;

begin
   TempString1 := StrAlloc(length(Source)+1);
   StrPCopy(TempString1, Source);

   TempString2 := StrAlloc(length(Dest)+1);
   StrPCopy(TempString2, Dest);

   New(Buffer);

   if Buffer = nil then
      begin
      CopyFile := fcNotEnoughMemory;
      Exit;
      end;

   if not FileExists(Source) then
      begin
      CopyFile := fcFileNotFound;
      Exit;
      end;

   if FileExists(Dest) and not OverWriteFile then
      begin
      CopyFile := fcFileExists;
      Exit;
      end;

   inFileHandle  := OpenFile(TempString1, FileStruct1, OF_READWRITE);
   outFileHandle := OpenFile(TempString2, FileStruct2, OF_CREATE +
OF_READWRITE + OF_SHARE_DENY_WRITE);

   done := False;

   BytesToRead := BufferSize;

   TotalFileSize := GetFileSize(Source);
   TotalBytes    := TotalFileSize;

   if BytesToRead > TotalFileSize then
      BytesToRead := TotalFileSize;

   while not done do
   begin
      rc := FileRead(inFileHandle, Buffer, BytesToRead);
      if rc <> BytesToRead then
         begin
         CopyFile := fcReadError;
         Exit;
         end;

               rc := FileWrite(outFileHandle, Buffer, BytesToRead);
         if rc <> BytesToRead then
            begin
            CopyFile := fcWriteError;
            Exit;
            end;
      TotalBytes := TotalBytes - BytesToRead;
      if TotalBytes <= 0 then
         begin
         done := True;
         end
      else
        if BytesToRead > TotalBytes then
           begin
           BytesToRead := TotalBytes;
           end;
   end;
   FileClose(inFileHandle);
   FileClose(outFileHandle);
   StrDispose(TempString1);
   StrDispose(TempString2);
   Dispose(Buffer);
end;

Brien King
bk...@primenet.com

 

Re:What am I doing Wrong in this Code?


In article <3157d5e5.1715060@news>, bk...@primenet.com says...
Quote

>Ok, I'm starting to pull my hair out.  Could someone please tell me
>what I'm doing wrong?  Any help would be greatly appreciated.  Its
>trashing memory somehow, I just don't see where.  (When run, it causes
>Win95 to drop to a command prompt with "invalid COMMAND.COM" error,
>not very pleasant)

Haven't spotted a specific error to explain the problem, but there's
a lot of room for improvement here.  After memory is allocated (StrAlloc,
New, OpenFile) there a number of places where you just bail out, leaving
all that memory dangling.  (This is Delphi, right?)  This stuff should be
be protected using try...finally to ensure that the memory is returned,
and you need to organize your code to always reach the deallocation
statements.  You might consider raising exceptions instead of returning
an error value; that would help by automatically calling any *finally*
sections and freeing memory, without requiring many changes to the overall
structure.  Otherwise, IMHO you'd need to use a multilevel *if* structure
to ensure reaching the end.

Perhaps you'd be better off using TFileStream or other Delphi-provided
means to do this.

On a larger scale, how are you going to have a robust application if your
low level routines are written this way?  Delphi provides a lot of valuable
tools for crash- and user-proofing programs, and they need to be built in
at the lowest level to work.  Good luck. :-)

Ken

Quote
>function CopyFile(Source, Dest: string; OverWriteFile, MoveFile:
>boolean): integer;

>const BufferSize = 32000;

>type BufferType = array[1..BufferSize] of Byte;

>var inFileHandle, outFileHandle: longint;
>    Buffer: ^BufferType;
>    BytesToRead, TotalBytes, TotalFileSize: longint;
>    TempString1, TempString2: PChar;
>    FileStruct1, FileStruct2: TOfStruct;
>    rc: longint;
>    done: boolean;

>begin
>   TempString1 := StrAlloc(length(Source)+1);
>   StrPCopy(TempString1, Source);

>   TempString2 := StrAlloc(length(Dest)+1);
>   StrPCopy(TempString2, Dest);

>   New(Buffer);

>   if Buffer = nil then
>      begin
>      CopyFile := fcNotEnoughMemory;
>      Exit;
>      end;

>   if not FileExists(Source) then
>      begin
>      CopyFile := fcFileNotFound;
>      Exit;
>      end;

>   if FileExists(Dest) and not OverWriteFile then
>      begin
>      CopyFile := fcFileExists;
>      Exit;
>      end;

>   inFileHandle  := OpenFile(TempString1, FileStruct1, OF_READWRITE);
>   outFileHandle := OpenFile(TempString2, FileStruct2, OF_CREATE +
>OF_READWRITE + OF_SHARE_DENY_WRITE);

>   done := False;

>   BytesToRead := BufferSize;

>   TotalFileSize := GetFileSize(Source);
>   TotalBytes    := TotalFileSize;

>   if BytesToRead > TotalFileSize then
>      BytesToRead := TotalFileSize;

>   while not done do
>   begin
>      rc := FileRead(inFileHandle, Buffer, BytesToRead);
>      if rc <> BytesToRead then
>         begin
>         CopyFile := fcReadError;
>         Exit;
>         end;

>               rc := FileWrite(outFileHandle, Buffer, BytesToRead);
>         if rc <> BytesToRead then
>            begin
>            CopyFile := fcWriteError;
>            Exit;
>            end;
>      TotalBytes := TotalBytes - BytesToRead;
>      if TotalBytes <= 0 then
>         begin
>         done := True;
>         end
>      else
>        if BytesToRead > TotalBytes then
>           begin
>           BytesToRead := TotalBytes;
>           end;
>   end;
>   FileClose(inFileHandle);
>   FileClose(outFileHandle);
>   StrDispose(TempString1);
>   StrDispose(TempString2);
>   Dispose(Buffer);
>end;

>Brien King
>bk...@primenet.com

--
Ken Irving                ...eschew obfuscation
SysTech Control
Fairbanks  AK  

Re:What am I doing Wrong in this Code?


Quote
bk...@primenet.com (Brien King) wrote:

Maybe you should change your code to BlockRead/BlockWrite.
Check out the Online-Help for exact syntax.

Another problem is: all those wonderful EXIT-Statements keep the
32K-Buffer allocated. You MUST delete this buffer before exiting,
otherwise your memory will go down VERY fast. Due to the lots of
FreeMem-Statements, use a Label and jump there with GOTO (IMHO this is
the ONE AND ONLY acceptable case, where I use a GOTO-Statement in
Pascal. No flames, please...)
I also think, it is better to use GETMEM and assign this to a Pointer.
Check out my changes:

Quote
>function CopyFile(Source, Dest: string; OverWriteFile, MoveFile:
>boolean): integer;
>const BufferSize = 32000;

label Xit;

Quote
>type BufferType = array[1..BufferSize] of Byte;

 {Remove the type}

Quote
>var inFileHandle, outFileHandle: longint;

      inFile, outFile: File;
Quote
>    Buffer: ^BufferType;

      Buffer: Pointer;

Quote
>    BytesToRead, TotalBytes, TotalFileSize: longint;
>    TempString1, TempString2: PChar;
>    FileStruct1, FileStruct2: TOfStruct;
>    rc: longint;
>    done: boolean;
>begin
>   TempString1 := StrAlloc(length(Source)+1);
>   StrPCopy(TempString1, Source);
>   TempString2 := StrAlloc(length(Dest)+1);
>   StrPCopy(TempString2, Dest);
>   New(Buffer);

     Getmem(Buffer,Buffersize);  {or sth like that :) }

Quote
>   if Buffer = nil then
>      begin
>      CopyFile := fcNotEnoughMemory;
>      Exit;
>      end;
>   if not FileExists(Source) then
>      begin
>      CopyFile := fcFileNotFound;
        goto Xit;      
>      end;
>   if FileExists(Dest) and not OverWriteFile then
>      begin
>      CopyFile := fcFileExists;
        goto Xit;
>      end;
>   inFileHandle  := OpenFile(TempString1, FileStruct1, OF_READWRITE);
>   outFileHandle := OpenFile(TempString2, FileStruct2, OF_CREATE +
>OF_READWRITE + OF_SHARE_DENY_WRITE);

     {Use ASSIGN and RESET/REWRITE here}

Quote
>   done := False;
>   BytesToRead := BufferSize;
>   TotalFileSize := GetFileSize(Source);
>   TotalBytes    := TotalFileSize;
>   if BytesToRead > TotalFileSize then
>      BytesToRead := TotalFileSize;
>   while not done do
>   begin
>      rc := FileRead(inFileHandle, Buffer, BytesToRead);

        BlockRead(inFile,Buffer,BytesToRead,rc);

{if BytesToRead <> rc then EOF is reached. If RC=0, an Error has
occured. Delphi's Exceptionhandling possibly signed up already}

Quote
>      if rc <> BytesToRead then
>         begin
>         CopyFile := fcReadError;

           goto Xit;

Quote
>         end;
>         rc := FileWrite(outFileHandle, Buffer, BytesToRead);

           BlockWrite(outFile,Buffer,rc,BytesToRead);  
Quote
>         if rc <> BytesToRead then
>            begin
>            CopyFile := fcWriteError;

              goto Xit;
Quote
>            end;
>      TotalBytes := TotalBytes - BytesToRead;
>      if TotalBytes <= 0 then
>         begin
>         done := True;
>         end
>      else
>        if BytesToRead > TotalBytes then
>           begin
>           BytesToRead := TotalBytes;
>           end;
>   end;

     CopyFile:=fcAllOK;  {Set proper ResultCode}
Xit:
Quote
>   FileClose(inFileHandle);
>   FileClose(outFileHandle);
>   StrDispose(TempString1);
>   StrDispose(TempString2);
>   Dispose(Buffer);

     FreeMem(Buffer,BufferSize);

Quote
>end;

Hope I could help ya,

Olaf
Ich liebe ISDN... Manchmal jedenfalls.

Other Threads