Re: [AD] Clarifying and revising filesystem API

[Thomas Fjellstrom <tfjellstrom@xxxxxxxxxx>]

On Wed April 15 2009, Peter Wang wrote:
> Here is an attempt to clarify the filesystem API, and mold it to my
> liking (mostly just superficial renaming; "entry" was far too generic,
> and the C/Unix-derived names seemed out of place).  I'm not finished
> with it, but it's time to sleep.  Some questions are marked with (Q).
> Suggestions welcome.
>
>
> =========================================================================
>
> % File system hooks
>
> # File system types
>
> ## API: ALLEGRO_FILE_HANDLE

No. Its not a file. Its an entry. This will just confuse people. Its generic 
for a reason.

> An opaque filesystem handle pointing to some path on the file system,
> either a regular file or a directory.
>
> Note that handles can be created whether the paths already exist or not. 
> You then use [al_open_file_handle] to actually open the file or directory
> for reading or writing.  Alternatively, use [al_fopen] to create a file
> handle and open a regular file in one step.
>
> See also: [al_open_file_handle], [al_fopen], [al_file_handle_is_regular],
> [al_file_handle_is_directory].
>
[snip]
> # Search Path Routines
>
> *(Q) As discussed, I think these should be removed*

I disagree.

> ## API: al_add_search_path
>
> Adds a path to the list of directories to search for files when
> searching/opening files with a relative pathname.
>
>
> ## API: al_get_search_path
>
> Fills in 'dest' up to 'len' bytes with the 'idx'th search path item.
>
> Parameters:
> idx - index of search path element requested
> dest - memory buffer to copy path to
> len - length of memory buffer
>
> Returns true on success, and false on failure.
>
> errno is set to indicate the error.
>
> Possible Errors:
> * EINVAL - invalid item selected
> * ERANGE - buffer not large enough
>
> See also: [al_get_errno]
>
>
> ## API: al_search_path_count
>
> Returns the number of items in the search path list.
>
>
>
[snip]
> ## API: al_close_dir
>
> *(Q) Why not just use al_close_file_handle or al_destroy_file_handle?*

With the orignal api it provides balance. open_dir/close_dir. but close_entry 
works just as well.

> Closes a previously opened directory entry object.
>
> [al_close_file_handle] is also a valid way to close any entry object.
>
> Does not free the entry object if it was opened with [al_open_dir].
> XXX This is probably a bug.
>
> Returns true on succes, false on failure and fills in errno to indicate the
> error.
>
>
[snip]
>
> ## API: al_remove_file
>
> *(Q) Do we need this?*

I was worried people would be against having a bunch of handles when all they 
really wanted to do was pass a string. But since people have had nothing 
against using ALLEGRO_PATHs and ENTRYs all over, many of the string versions 
of functions can probably go away.

> Like [al_remove_file_handle] but takes the path as a string rather than a
> handle representing the path.
>
>
> ## API: al_update_file_handle_info
>
> *(Q) Does this need to be called before al_get_file_handle_mode, etc.?*

Of course not. the handle creation does an initial stat and fills everything 
in. Its only needed if you NEED to update the info.

> Update the status information for the given filesystem handle,
> i.e. modes, timestamps, etc.
>
> Returns true on success, false on failure.
> On an error, the Allegro errno will be set.
>
> See also: [al_get_errno], [al_get_file_handle_atime],
> [al_get_file_handle_ctime], [al_is_directory], [al_is_file],
> [al_get_file_handle_mode].
>
[snip]
>
> ## API: al_fclose
>
> *(Q) Why not just make this a synonym for al_destroy_file_handle?*

If you feel thats ok, the only reason I'd argue against it is because its a 
direct interface to the vtable, and a given fshook driver may want to 
implement them differently. for whatever reason.

> Closes the given file entry object.
> Will destroy the handle if it was opened with al_fopen.
>
> If you do not wish the entry object destroyed, use [al_close_file_handle]
> instead.
>
>
[snip]
>
> ## API: al_change_directory
>
> Changes the current working directory to 'path'.
>
> Returns true on success, false on error.
>
> *(Q) need to figure out what to do with this if there are multiple
> filesystems in effect; which filesystem's version of the "current
> directory" is changed?*

Theres also a concept as current drive in windows, so that fixes that.

> *(Q) need to consider if this should _actually_ change the process'
> working directory when using stdio driver, or just change it logically*

Why bother storing our own current dir? the OS already provides it for us.

> *(Q) threads*
>
>
> ## API: al_get_path_separator
>
> *(Q) need to figure out what to do with this if there are multiple
> filesystems in effect, each with potentially its own syntax*

Huh? I don't follow. Why would multiple file systems on a host have different 
syntax? Besides the intention is for fshooks to follow unix file system 
syntax, and convert back and forth if needed (inside the driver).

> Fills in 'sep' up to 'len' characters with the path separator string.
> XXX return code?
>


That said, what I suggested and implemented is a lowish level interface. Its 
not meant to do anything fancy. What its here to do is make it easy for the 
user and allegro to share the same file system functions. Not all users will 
want to be stuck with pack_* or stdio, they may want to use zlib, or physfs, 
or even something like KIO. This allows allegro to be told how to work with 
what the user wants to work with. It is NOT meant to be a full featured VFS. 
This is why theres no uri or network support. You want to open zips? Wire up a 
library that hooks into allegro to provide it. fshooks itself does not, and 
should not provide any of that.

-- 
Thomas Fjellstrom
tfjellstrom@xxxxxxxxxx