| Re: [AD] Clarifying and revising filesystem API |
[ Thread Index |
Date Index
| More lists.liballeg.org/allegro-developers Archives
]
Peter Wang wrote:
* ALLEGRO_MAKE_TEMP_REMOVE_ON_OPEN
That confuses me a bit... wouldn't it make more sense to say CLEAR_ON_OPEN?
*(Q) As discussed, I think these should be removed*
I still think so. Let's leave this to the physfs addon (and it will be up to the
addon author whether to hide physfs and provide wrappers for the search path
stuff, or have users call physfs functions and just provide some glue).
*(Q) Why not just use al_close_file_handle or al_destroy_file_handle?*
*(Q) At least in the stdio version, this is no different to
al_open_file_handle followed by a check that the path was a directory, so
maybe this is not required?*
There may be a reason - but right now, I also don't see it. If an
ALLEGRO_FILE_ENTRY is a directory, opening/closing it should always be the same
as al_open/close_dir. So those are indeed not needed.
*(Q) Peter Hull points out there is no rewind.*
We could make al_fseek work with it. And independent of that an al_rewind()
which just does al_fseek(0) does not sound bad either.
## API: al_remove_file
*(Q) Do we need this?*
Why not. If my game removes a save-file, I probably have the location available
as a path... so this makes it easier than having to construct a file handle first.
## API: al_update_file_handle_info
*(Q) Does this need to be called before al_get_file_handle_mode, etc.?*
I'd say no. Those functions should check a flag to see if stat() info is
available and if not call it. The only use for al_update_file_handle_info would
be if you suspect something might have changed since the last time you called
any of those functions (and therefore stat was called and the info cached).
## API: al_file_exists
*(Q) I think this is unnecessary*
We already have al_path_exists, so yeah. One question is, would those two always
have done the same?
## API: al_file_is_regular
*(Q) I think this is unnecessary*
## API: al_file_is_directory
*(Q) I think this is unnecessary*
## API: al_get_file_mode
*(Q) I think this is unnecessary*
## API: al_get_file_atime
*(Q) I think this is unnecessary*
## API: al_get_file_ctime
*(Q) I think this is unnecessary*
## API: al_get_file_mtime
*(Q) I think this is unnecessary*
## API: al_get_file_size
*(Q) I think this is unnecessary*
Not sure, but they all seem uncommon enough that you could just as well create a
file handle and query the info using that.
## API: al_fclose
*(Q) Why not just make this a synonym for al_destroy_file_handle?*
Or remove it completely and rename al_fopen to al_open_file.
## API: al_feof
Returns true if we have an end of file condition.
I'd like a note here to explicitly define the behavior when fungetc was used.
## API: al_fgets
I think there's another thread about it and it's still unresolved.
*(Q) Consider naming this with 'byte' in the name, as char != byte since
forever.*
Indeed. I would be fine with al_read_byte, libc names tend to be rather cryptic.
## API: al_fread_32le
Reads a 32-bit word in little-endian format (LSB first).
Returns:
The read 32-bit word or EOF on error.
I think this should be:
"""
Returns the read 32-bit word. If an error occurs the return value is undefined.
"""
The reason is, we apparently can't return anything here which isn't also a valid
return value. So we should not tempt the user to check for EOF or 0 or whatever
before assuming an error (and consequently forget to do proper error checking).
## API: al_fwrite_32le
Writes a 32-bit word in little-endian format (LSB first).
Returns:
The written 32-bit word or EOF on error.
I would prefer this to either return void or the number of bytes written (which
would be either 4, or 0 on error).
## API: al_fseek
## API: al_ftell
We could make those work with directories (in the stdio driver, use "seekdir"
and "telldir" from libc). Or if not, we probably should state that it's an error
to call them on non-file entries.
## 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?*
*(Q) need to consider if this should _actually_ change the process'
working directory when using stdio driver, or just change it logically*
*(Q) threads*
Yeah. This is very closely related to the search path issue of course. *If* we
keep a logical current directory anyway, then it actually would be tempting to
allow an addon to do something like:
al_zip_set_current_archive("data.zip")
And then all paths would be interpreted to inside the zip file... I feel we
discuss the same thing on multiple places now though...
## 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*
How relevant is the path separator btw.? Would things break if we always use /?
I found it a bit cumbersome to have to use:
al_path_to_string(path, al_get_path_separator())
Maybe we chould have a function like this:
ALLEGRO_FILE_HANDLE *al_create_file_handle_from_path(ALLEGRO_PATH *path)
--
Elias Pschernig <elias@xxxxxxxxxx>