Should I raise an exception when getting the size or the position of a file?-CodePudding

I'm trying to make a class in Delphi that handles files. I have a property that returns the size of the file and another one that returns the position of the file. I don't know if any error can happen with these calls. Should I raise an exception?

My code is:

function TFile.GetSize: Int64;
var
  FileSizeHi, FileSizeLo: Cardinal;
begin
  FileSizeLo := GetFileSize(FHandle, @FileSizeHi);
  if (FileSizeLo = INVALID_FILE_SIZE) and (GetLastError = NO_ERROR) then
    Result := $FFFFFFFF
  else
    Result := FileSizeLo or Int64(FileSizeHi) shl 32;
end;

function TFile.GetPosition: Int64;
var
  FilePosHi, FilePosLo: Cardinal;
begin
  FilePosHi := 0;
  FilePosLo := 0;
  FilePosLo := SetFilePointer(FHandle, FilePosLo, @FilePosHi, FILE_CURRENT);
  if (FilePosLo = INVALID_SET_FILE_POINTER) and (GetLastError = NO_ERROR) then
    Result := $FFFFFFFF
  else
    Result := FilePosLo or Int64(FilePosHi) shl 32;
end;

I don't know what error could happen when I call GetFileSize or SetFilePointer (without moving the file pointer).

CodePudding user response：

These are WinAPI calls and you are already checking the return values, so you should be good to go. It is up to the user of your class to check for the return value from your functions. For more information check SetFilePointer() and GetFileSize().

CodePudding user response：

Yes, errors can happen with those functions, so I would recommend raising an exception, otherwise the caller doesn't know if it has received an invalid value or not, as $FFFFFFFF is a valid size/position for 64bit values. Perhaps you meant to use -1 ($FFFFFFFFFFFFFFFF) instead?

However, whether you raise an exception or not, your GetLastError() check is wrong. It needs to use <> instead of =. When the file function returns $FFFFFFFF for the low value, GetLastError() will return 0 when the low value really is $FFFFFFFF when the high value is non-zero, otherwise GetLastError() will return non-zero when the low/high values are invalid.

Try this:

function TFile.GetSize: Int64;
var
  FileSizeHi, FileSizeLo: DWORD;
begin
  FileSizeLo := GetFileSize(FHandle, @FileSizeHi);
  if (FileSizeLo = INVALID_FILE_SIZE) and (GetLastError <> NO_ERROR) then
    RaiseLastOSError // or: Result := -1
  else
    Result := FileSizeLo or (Int64(FileSizeHi) shl 32);
end;

function TFile.GetPosition: Int64;
var
  FilePosHi, FilePosLo: DWORD;
begin
  FilePosHi := 0;
  FilePosLo := 0;
  FilePosLo := SetFilePointer(FHandle, FilePosLo, @FilePosHi, FILE_CURRENT);
  if (FilePosLo = INVALID_SET_FILE_POINTER) and (GetLastError <> NO_ERROR) then
    RaiseLastOSError // or: Result := -1
  else
    Result := FilePosLo or (Int64(FilePosHi) shl 32);
end;

On a side note, consider using GetFileSizeEx() and SetFilePointerEx() instead, as they operate on 64bit values without breaking them up into low/high parts.