bmx-ng / brl.mod

BlitzMax Runtime Libraries, for BlitzMax NG.
12 stars 12 forks source link

[brl.FileSystem] DeleteFile() and CreateDir() return values "incorrect" #226

Open GWRon opened 2 years ago

GWRon commented 2 years ago

DeleteFile() and CreateDir() return their values according to the passed parameter - not corresponding to the desired "action".

Assume you misspell a filename: existing: "my_file_here.txt" command: Delete("myfilehere.txt") the result of the command will be True because the file "myfilehere.txt" is not existing at the end of command (FileType(path)=FILETYPE_NONE) - yet it never existed before. Delete() should return what the actual deletion commands "return" as results

Similar for CreateDir() existing: "DataFolder" command CreateDir("DataFolder") the resolut of the command will be True because FileType(path)=FILETYPE_DIR evaluates to true - yet it is more of interest if the directory was actually created or not. This one here is only a minor "bug" - as it does not result in "skipping" something - so final result also results in the directory existing or not.

current implementation for easier lookup:

Function DeleteFile:Int( path$ )
    FixPath path
    If MaxIO.ioInitialized Then
        MaxIO.DeletePath(path)
    Else
        remove_ path
    End If
    Return FileType(path)=FILETYPE_NONE
End Function

Function CreateDir:Int( path$,recurse:Int=False )
    FixPath path,True
    If MaxIO.ioInitialized Then
        Return MaxIO.MkDir(path)
    Else
        If Not recurse
            mkdir_ path,1023
            Return FileType(path)=FILETYPE_DIR
        EndIf
        Local t$
        path=RealPath(path)+"/"
        While path
            Local i:Int=path.find("/")+1
            t:+path[..i]
            path=path[i..]
            Select FileType(t)
            Case FILETYPE_DIR
            Case FILETYPE_NONE
                Local s$=StripSlash(t)
                mkdir_ StripSlash(s),1023
                If FileType(s)<>FILETYPE_DIR Return False
            Default
                Return False
            End Select
        Wend
        Return True
    End If
End Function

Thanks to @davecamp for accidentally spotting it :)

GWRon commented 2 years ago

Ok, there is another minor issue - which I am not sure how to properly "do it".

physfs mentions for physfs_delete:

Note that on Unix systems, deleting a file may be successful, but the actual file won't be removed until all processes that have an open filehandle to it (including your program) close their handles. Chances are, the bits that make up the file still exist, they are just made available to be written over at a later point. Don't consider this a security method or anything. :)

So .. when removing a file ... what do we expect? Should it return "true" only if the file is no longer there - or if the "delete request" was successful ?

GWRon commented 2 years ago

We could consider introducing the error codes of physfs to there ... so you see if the file was actually deleted, the deletion request was successful (so file will be removed "somewhen"), ...

For now the current implementation just returns wether your "intention" was fulfilled - not if your "request" was doing something.