Recent

Author Topic: Safe way to access files  (Read 688 times)

torbente

  • Sr. Member
  • ****
  • Posts: 325
    • Noso Main Page
Safe way to access files
« on: August 07, 2022, 01:38:53 am »
Hi

Just trying to figure a safe way to access files in a simple and standarized way, i found 2 different approaches:

Using finally:

Code: Pascal  [Select][+][-]
  1. Procedure RunTest();
  2. var
  3.   LFile : TextFile;
  4.   Fiop  : boolean = false;
  5. Begin
  6. Assignfile(LFile,'test.dat');
  7. TRY
  8.    TRY
  9.    Reset(LFile);
  10.    Fiop := true;
  11.    {do stuff with file}
  12.    EXCEPT ON E:Exception do
  13.       ToLog(' Error Handling');
  14.    END;
  15. FINALLY
  16. if Fiop then CloseFile(LFile);
  17. END;
  18. End;

Except + Except blocks

Code: Pascal  [Select][+][-]
  1. Procedure RunTest();
  2. var
  3.   LFile : TextFile;
  4. Begin
  5. Assignfile(LFile,'test.dat');
  6. TRY
  7. Reset(LFile);
  8.    TRY  
  9.    {do stuff with file}
  10.    EXCEPT ON E:Exception do
  11.       ToLog(' Error handling');
  12.    END;
  13. CloseFile(LFile);
  14. EXCEPT ON E:Exception do
  15.    ToLog(' Error accessing');
  16. END;
  17. End;

My question is: is any of this approaches safe? What is the best one? Are there something im missing? Second one seems "safer" IMHO since it closes the file inside a try block, and AFAIK guarantees that the file will be always closed after the procedure finish.

Thanks in advance
« Last Edit: August 07, 2022, 01:42:41 am by torbente »
Noso Cryptocurrency Main Developer
https://github.com/DevTeamNoso/NosoWallet

dje

  • Full Member
  • ***
  • Posts: 111
Re: Safe way to access files
« Reply #1 on: August 07, 2022, 01:56:57 am »
Neither. Use TStream derived access.
You can either use old style records and procedures:
Code: Pascal  [Select][+][-]
  1. type
  2.   TMyDataRecord = record
  3.   end;
  4.  
  5. procedure MyDataLoadFromStream(var AData: TMyDataRecord; AStream: TStream);
  6. begin
  7.   // Write code to load data from stream
  8. end;
  9.  
  10. procedure MyDataLoadFromFile(var AData: TMyDataRecord; const AFileName: TFileName);
  11. var
  12.   LStream: TStream;
  13. begin
  14.   LStream := TFileStream.Create(AFileName, fmOpenRead);
  15.   try
  16.     MyDataLoadFromStream(AData, LStream);
  17.   finally
  18.     FreeAndNil(LStream);
  19.   end;
  20. end;  

Or, use "new" style objects or classes:
Code: Pascal  [Select][+][-]
  1. type
  2.  
  3.   TMyData = object (* or class derived from tobject/tcomponent/etc *)
  4.     procedure LoadFromStream(AStream: TStream);
  5.     procedure LoadFromFile(const AFileName: TFileName);
  6.   end;
  7.  
  8. procedure TMyData.LoadFromStream(AStream: TStream);
  9. begin
  10.   // Write code to load data from stream
  11. end;
  12.  
  13. procedure TMyData.LoadFromFile(const AFileName: TFileName);
  14. var
  15.   LStream: TStream;
  16. begin
  17.   LStream := TFileStream.Create(AFileName, fmOpenRead);
  18.   try
  19.     LoadFromStream(LStream);
  20.   finally
  21.     FreeAndNil(LStream);
  22.   end;
  23. end;

The tips being:
1) Encapsulate your data in a data structure.
2) Write a LoadFromStream() procedure to load data from the abstract TStream class.
3) Write a helper procedure which will create a TFileStream and call LoadFromStream(). (With exception handling)


dje

  • Full Member
  • ***
  • Posts: 111
Re: Safe way to access files
« Reply #2 on: August 07, 2022, 02:02:36 am »
But... If you must use the old style Pascal file functions, then, _always_ separate your file reading code from your file setup code. Note: I always turn off Pascal file error exceptions, and write my own ioresult checking. Its up to you.

Code: Pascal  [Select][+][-]
  1. procedure MyDataLoadFrom(var AData: TMyDataRecord; var AFile: TextFile);
  2. begin
  3.   // Write code to load data from textfile
  4. end;
  5.  
  6. procedure MyDataLoadFrom(var AData: TMyDataRecord; const AFileName: TFileName);
  7. var
  8.   LFile: TextFile;
  9. begin
  10.   AssignFile(LFile, AFileName);
  11.   Reset(LFile);
  12.   if IOResult = 0 then begin
  13.     try
  14.       MyDataLoadFrom(AData, LFile);
  15.     finally
  16.       Close(LFile);
  17.     end;
  18.   end else begin
  19.     raise Exception.Create('Failed load');
  20.   end;
  21. end;  
« Last Edit: August 07, 2022, 02:04:22 am by derek.john.evans »

torbente

  • Sr. Member
  • ****
  • Posts: 325
    • Noso Main Page
Re: Safe way to access files
« Reply #3 on: August 07, 2022, 02:21:06 am »
@derek.jhon.evans

Thanks for your replys.

You used one word, "neither", to ask my question, and then 2 posts to submit sugerences that implicates change the way how the existing application manage the data. Im sure your methods are better, but i still have no clue why neither of the proposed ways is good and why.

PS Methods proposed are simple ways to guarantee that any issue openning the file will be catched, and the file will be close at finish, without crash the app.
« Last Edit: August 07, 2022, 02:23:42 am by torbente »
Noso Cryptocurrency Main Developer
https://github.com/DevTeamNoso/NosoWallet

Thaddy

  • Hero Member
  • *****
  • Posts: 11939
Re: Safe way to access files
« Reply #4 on: August 07, 2022, 11:31:49 am »
No, access rights to write to a file are the issue, not old school or new school.
The issue is that when you write to a file, nobody else should be able to interfere.
That means just the opposite of what you wrote: During writes, not reads, the data should be locked.

Although complex merges of data being eddited between multiple users are possible to some extend by buffering.
Which is what most SQL flavors do.
« Last Edit: August 07, 2022, 11:34:51 am by Thaddy »
Black themes should be banned.

MarkMLl

  • Hero Member
  • *****
  • Posts: 5213
Re: Safe way to access files
« Reply #5 on: August 07, 2022, 11:48:52 am »
No, access rights to write to a file are the issue, not old school or new school.
The issue is that when you write to a file, nobody else should be able to interfere.
That means just the opposite of what you wrote: During writes, not reads, the data should be locked.

Although complex merges of data being eddited between multiple users are possible to some extend by buffering.
Which is what most SQL flavors do.

Very much agree. However at this point I'd emphasise that OP *SHOULD* have told us what OS etc. he's using, since details differ.

On Windows, file locking and single-program access were designed-in and broadly speaking work.

On unix, locking etc. was an afterthought and in many cases is still either advisory or only partially-implemented.

If in doubt, use a database. And if there is any chance whatsoever of a situation where multiple systems need to access data simultaneously, then go straight for a database that has a network API: DO NOT, UNDER ANY CIRCUMSTANCES, ATTEMPT TO USE FILE-SHARING OPERATIONS ON A DATABASE'S STORAGE.

MarkMLl
MT+86 & Turbo Pascal v1 on CCP/M-86, multitasking with LAN & graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.
GitHub repositories: https://github.com/MarkMLl?tab=repositories

torbente

  • Sr. Member
  • ****
  • Posts: 325
    • Noso Main Page
Re: Safe way to access files
« Reply #6 on: August 07, 2022, 01:34:21 pm »
Hi,
Is a multiplatform situation (win/linux).
I use flags ,CriticalSections or only access from main thread to avoid multi access to the same file, so permissions are not an issue (not from the same app)

I would love to understand why the second example is not good in those situations.

Thanks again.
Noso Cryptocurrency Main Developer
https://github.com/DevTeamNoso/NosoWallet

dje

  • Full Member
  • ***
  • Posts: 111
Re: Safe way to access files
« Reply #7 on: August 08, 2022, 06:27:17 am »
@derek.jhon.evans
Thanks for your replys. You used one word, "neither", to ask my question, and then 2 posts to submit sugerences that implicates change the way how the existing application manage the data. Im sure your methods are better, but i still have no clue why neither of the proposed ways is good and why.

PS Methods proposed are simple ways to guarantee that any issue openning the file will be catched, and the file will be close at finish, without crash the app.

@derek.jhon.evans
You used one word, "neither", to ask my question, and then 2 posts to submit sugerences that implicates change the way how the existing application manage the data. Im sure your methods are better, but i still have no clue why neither of the proposed ways is good and why.

Well, OK. In general, try/except blocks should only be used to "catch and raise". So, your first example should be:
Code: Pascal  [Select][+][-]
  1. procedure RunTest;
  2. var
  3.   LFile: TextFile;
  4. begin
  5.   Assignfile(LFile, 'test.dat');
  6.   Reset(LFile);
  7.   try
  8.     {do stuff with file}
  9.   finally
  10.     CloseFile(LFile);
  11.   end;
  12. end;  

And you should call RunTest() with.

Code: Pascal  [Select][+][-]
  1.   try
  2.     RunTest;
  3.   except
  4.     ON E: Exception do ToLog('Error Handling');
  5.   end;

Note how I've moved the exception "catch" outside the procedure itself. That means RunTest() does not define how exceptions are handled, and therefore it makes the implementation reusable across projects. Note, how I also, don't re-raise the exception since I'm happy that it has been handled, but If I wanted to, I could without creating a file handle leak. Bear in mind, the ToLog() could also raise an exception, which would cause problems in your examples.

Your second example has the same problem of catching exceptions inside what should be a generic reusable function. In the event, someone adds a "raise" command to your exception catch, then the closefile() will be skipped. For simple functions, its easy to see the issue. For larger programs, its easy to miss.

So, the first rule of thumb is, when ever a non-managed resource is allocated, you _must_ add try/finally blocks. ie:
Code: Pascal  [Select][+][-]
  1.   Reset(LFile);
  2.   try
  3.     {do stuff with file}
  4.   finally
  5.     CloseFile(LFile);
  6.   end;

The second rule is to place the code for {do stuff with file} into its own procedure. This prevents the try/finally from becoming obscured by the "stuff". It also adds extra functionality by allowing code to use any file handle. ie: Input/Output or in the case of TSteam, compression, encryption, memory, network, etc streams.

And the third rule is to catch exceptions outside of the "functionality" code. Exceptions were created as an improved way to pass errors back to calling functions.

Follow these guidelines and you will generate clear, concise & bulletproof source code.

Edit: Also note, Pascal file commands arnt guaranteed to raise exceptions, so IOResult should be checked. But, I think Ive said enough :-)
« Last Edit: August 08, 2022, 06:37:26 am by derek.john.evans »

MarkMLl

  • Hero Member
  • *****
  • Posts: 5213
Re: Safe way to access files
« Reply #8 on: August 08, 2022, 09:23:31 am »
Also note, Pascal file commands arnt guaranteed to raise exceptions, so IOResult should be checked. But, I think Ive said enough :-)

Noting that that might need a directive override to work

Code: Pascal  [Select][+][-]
  1. function openInput(const fn: string): boolean;
  2.  
  3. begin
  4.   if fn = '' then
  5.     exit(false);
  6. {$push }
  7. {$i- }
  8.   AssignFile(pesIn, fn);
  9.   Reset(pesIn, 1);
  10.   result := IOResult = 0
  11. {$pop  }
  12. end { openInput } ;
  13.  

MarkMLl
MT+86 & Turbo Pascal v1 on CCP/M-86, multitasking with LAN & graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.
GitHub repositories: https://github.com/MarkMLl?tab=repositories

 

TinyPortal © 2005-2018