Re: g_file_test()

[Tim Janik <timj gtk org>]

On 3 Sep 2001, Owen Taylor wrote:

> Tim Janik <timj gtk org> writes:
> 
> > On Mon, 6 Aug 2001, Tim Janik wrote:
> > 
> > > 
> > > typedef enum
> > > {
> > >   G_FILE_TEST_IS_REGULAR    = 1 << 0,
> > >   G_FILE_TEST_IS_SYMLINK    = 1 << 1,
> > >   G_FILE_TEST_IS_DIR        = 1 << 2,
> > >   G_FILE_TEST_IS_EXECUTABLE = 1 << 3,
> > >   G_FILE_TEST_EXISTS        = 1 << 4
> > > } GFileTest;
> > > 
> > > gboolean g_file_test         (const gchar  *filename,
> > >                               GFileTest     test);
> > > 
> > > having used g_file_test() in a couple places now, i have to say that:
> > > 1) it definitely needs G_FILE_TEST_IS_READABLE and G_FILE_TEST_IS_WRITABLE
> > > 2) it should return TRUE only if all tests succeeded.
> > 
> > are there actually any objections/concerns not yet raised on this issue?
> > if not, i'd like to fix matters ASAP.
> 
> OK, the original discussion is at:
> 
>  http://mail.gnome.org/archives/gtk-devel-list/2000-October/msg00154.html
> 
> The primary concern that came up with making it all-of is that 
> that would conflict with the current semantics of g_file_test()
> in gnome-libs, which I think is 

first, and for the record, i strongly dislike the idea of gnome-libs introducing
random g_* stuff with broken semantics, and then relying on us to fold those
changes back into glib while keeping their broken semantics. i consider this
illegitimate namespace clutter. that being said, read below on how i think we
should handle the current situation.

> I agree that all-of is more what I would expect, but I don't
> think the any-of behavior is so ridiculous that I would be
> willing to declare programs that expect it buggy. I think if
> we change the behavior, we have to change the name.

grepping through my checked out GNOME CVS stuff, the only case where
OR semantics are currently being used is stuff like:

./gnome-libs/libgnomeui/gnome-icon-entry.c:     if(!p || !g_file_test (p,G_FILE_TEST_ISLINK|G_FILE_TEST_ISFILE) ||
./gnome-libs/libgnomeui/gnome-pixmap-entry.c:   if(!p || !g_file_test(p,G_FILE_TEST_ISLINK|G_FILE_TEST_ISFILE) ||
./nautilus/libnautilus-private/nautilus-theme.c: || !g_file_test (theme_to_install_path, G_FILE_TEST_EXISTS | G_FILE_TEST_ISDIR)) {
./control-center/capplets/background-properties/property-background.c:  if(!p || !g_file_test (p,G_FILE_TEST_ISLINK|G_FILE_TEST_ISFILE) ||
./control-center/capplets/file-types/file-types-icon-entry.c:   if(!p || !g_file_test (p,G_FILE_TEST_ISLINK|G_FILE_TEST_ISFILE)
./glib/gmodule/gmodule.c:      if (!g_file_test (file_name, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))

so there're three cases to isolate:
nautilus:   G_FILE_TEST_EXISTS | G_FILE_TEST_ISDIR	(old API)
gnome-libs: G_FILE_TEST_ISLINK | G_FILE_TEST_ISFILE	(old API)
glib:       G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR	(new API)

the test in nautilus is somewhat bogus, it isn't actually assuming OR semantics,
since if ISDIR succeeds, EXISTS also will. the author probably meant "file exists
and is a directory", i.e. AND semantics (though EXISTS is extraneous here).

the gnome-libs test for G_FILE_TEST_ISLINK | G_FILE_TEST_ISFILE is probably
the only one that code that authors would actually want to make with OR
semantics, these kinds of old checks will be catched with glib-2.0 though,
because we'ce changed the API to read G_FILE_TEST_IS_SYMLINK |
G_FILE_TEST_IS_REGULAR here (i.e. different flag names).
this is not a very usefull test though, since we're using stat(2) not
lstat(2) in g_file_test().

> (I'd think the reason why it's any-of is what you get when you
> read:
> 
>  g_file_test (G_FILE_TEST_IS_REGULAR | G_FILE_TEST_IS_SYMLINK);
> 
> Yep, "test file is regular or file is symlink".)

there's no real point in making this check:

lrwxrwxrwx    1 timj     cracks         20 Sep  4 03:52 dangling -> don't have this file
lrwxrwxrwx    1 timj     cracks          8 Sep  4 04:08 dangling-link -> dangling
-rw-r--r--    1 timj     cracks          0 Sep  4 03:52 regular-file
lrwxrwxrwx    1 timj     cracks         12 Sep  4 03:52 regular-link -> regular-file
file test for "dangling":
  is_regular: 0
  is_symlink: 0
  is_dir    : 0
  is_exec   : 0
  exists    : 0
file test for "dangling-link":
  is_regular: 0
  is_symlink: 0
  is_dir    : 0
  is_exec   : 0
  exists    : 0
file test for "regular-file":
  is_regular: 1
  is_symlink: 0
  is_dir    : 0
  is_exec   : 0
  exists    : 1
file test for "regular-link":
  is_regular: 1
  is_symlink: 0
  is_dir    : 0
  is_exec   : 0
  exists    : 1

> Possibilities for an all-of name include:
> 
>  g_file_test_is_all ()
>  g_file_test_all ()
>  g_file_is_all ()
>  g_file_is ()

if we did introduce g_file_test_all() for having AND semantics,
we should still remove g_file_test(), since most people will assume
AND semantics intuitively.
i don't think we'll introduce real problems with just changing g_file_test()
to have AND semantics though, the old API uses of G_FILE_TEST_ISLINK |
G_FILE_TEST_ISFILE will be caught and have to be changed anyways.
and in fact, we should get rid of G_FILE_TEST_IS_SYMLINK, since we don't use
lstat(2).

> I think G_FILE_TEST_IS_READABLE/IS_WRITEABLE are open to abuse, and
> its pretty hard to get them exactly right, considering things like:
> 
>  - read-only mounts
>  - Multiple groups for a user
>  - ACLs on AFS
>  - Behavior of access as root and with setuid
> 
> Basically, the only way to safely tell if you would be able
> to read/write a file is to actually do it. 
> 
> Does that mean we shouldn't add them? It may be that if we don't,
> people will write the tests themselves and do it worse.  I don't
> know...

read/write are no different to handle than the current
G_FILE_TEST_IS_EXECUTABLE which is done via acces(2).
so adding G_FILE_TEST_IS_READABLE and G_FILE_TEST_IS_WRITEABLE just
means adding R_OK and W_OK to the mask passed in to access(2) (which
also provides AND semantics already).

> When do you see them being useful?

it should be clear that code which reads from/writes to a file/device should
sanely handle errors already (EINTR, disk full and whatnot), but with
that in mind, adding readable/writable flags to the test mask if you have
to do g_file_test() anyways, can be quite convenient. e.g. to figure whether
a device or a directory contained in a PATH-alike variable is readily accessible
(and before handling it over to some lower layers which do the actual IO).

> 
>                                         Owen
> 

---
ciaoTJ