Mail Archives: djgpp-workers/2000/12/26/03:53:50
On Mon, 25 Dec 2000, Tim Van Holder wrote:
> Below is a patch to extend opendir() et al to support 'virtual
> files' introduced by the fsext mechanism (eg /dev/zero). The
> idea is that fsext modules register and deregister any files
> they handle.
Thanks for working on this.
> The registering/deregistering isn't yet implemented in this
> draft, as this probably requires more thought: i.e. do fsext
> functions know the name of the file they're working on after
> the initial open()?
I send a few comments about the code below, but I suggest first to
invest the thought you mention above, before plunging into the
implementation details. It is hard to reflect on code before dust
settles on important API and usage considerations.
Here are some issues I suggest to discuss:
- Why would an application want to fake directory entries, and for
what kind of functionality? Until now, we have been talking about
/dev/* pseudo-files only, which don't need such a general-purpose
mechanism.
- In what kind of data structure(s) will the fake entries be stored?
- How would an application register and deregister entries in
practice? In what directory (or directories) will those entries be
visible, and what API does an application need to control this?
- Can fake entries be registered before an opendir call for their
parent directory? Can they be deregistered before closedir is
called?
- Will the fake entries appear in the beginning or the end of the
real entries?
- Why do we need to invent a special mechanism? Isn't it enough to
define an __FSEXT_dir hook that would be called by all these
functions, in the same manner as __FSEXT_open hook is called by
_open?
> This patch also fixes a bug in telldir/seekdir.
> telldir() returns the number of actual entries read and
> seekdir() calls readdir() that many times after doing a
> rewinddir(). But if the DIR has simulated dots (or with this
> patch, other simulated entries), those numbers do not match.
Please don't mix patches for different problems together. Please send
the telldir patches separately.
Anyway, I'm not sure what is the bug you are trying to solve. Can you
give an example of a series of calls to opendir, readdir, telldir, and
seekdir where the current implementation fails?
> + char**
> + __fsext_entries (const char* d)
> + {
> + return NULL;
> + }
It is not clear why do you need this function. Since the fake entries
are stored in an array accessible from the DIR structure, what do you
need __fsext_entries for?
> + int exists;
> + char **fsext_entries;
> + char **fsext_entry;
> };
Please explain the need for the `exists' member.
I also don't understand why do you need both fsext_entries and
fsext_entry, since they both are used to index into the same array.
Isn't num_read enough for that?
> ! /* Ensure that directory to be accessed exists; only matters if there are
> ! * no entries faked by the fsext system. */
> if (access(dir->name, D_OK))
> {
> ! if (dir->fsext_entries == NULL)
> ! {
> ! free(dir->name);
> ! free(dir);
> ! return 0;
> ! }
> ! else
> ! {
> ! dir->exists = 0;
> ! dir->need_fake_dot_dotdot = 2;
> ! return dir;
> ! }
I don't understand this: why do you keep reporting data to the
application if the directory doesn't exist? This code is about fake
directory entries, not about fake directories, isn't it?
Also, how is the fact that dir->fsext_entries is non-NULL relevant to
whether the directory may or may not be non-existent?
> ! if (dir->exists == 2)
> done = findnext(&dir->ff);
> else
> {
> ***************
> *** 39,44 ****
> --- 53,59 ----
> if (dir->flags & __OPENDIR_FIND_LABEL)
> ff_flags |= FA_LABEL;
> done = findfirst(dir->name, &dir->ff, ff_flags);
> + dir->exists = 2;
What is the special meaning of dir->exists == 2? If this is not a
simple boolean variable, I think the name `exists' is inappropriate.
- Raw text -