*** This bug has been marked as a duplicate of 167941 ***

[ Sorry, too fast ]

It seems that on sparc and ppc the modification time of the dir is set to 0 when
gtk-update-icon-cache is used. In gtkicontheme.c mtime == 0 is used to indicate
that a dir doesn't exist (see line 199). Thus because the dir's mtime == 0
gtkicontheme thinks it doesn't exists, causing the problem..

Now why replacing all g_stat() calls by stat() fixes the issue, but is not clear
to me why that helps as g_stat just calls stat()...

> Now why replacing all g_stat() calls by stat() fixes the issue, but is not clear
> to me why that helps as g_stat just calls stat()...

The Debian packages have a patch that does this, but it doesn't seem to help on
powerpc.

BTW, the generated icon-theme.cache files on my powerpc machine are no longer
identical to the ones on my x86 machine. Should they be? FWIW, copying a file
from x86 to powerpc doesn't seem to work either, then again I'm not sure I'm
doing all that's needed to make sure the new file is actually picked up...

I know, but it only changes one g_stat to stat.. That fixed one problem on sparc.

The important g_stat for this problem is the one on line 606 of
updateiconcache.c, which determines the mtime of the directory..

Indeed, changing line 606 from g_stat to stat works. Looks like g_stat is
broken? (It seems to always return 4096 for utime_buf.actime as well)

I would guess this is a largefile issue. glib is compiled with AC_SYS_LARGEFILE,
thus it uses stat64. If you pass it a struct stat, things probably break. 

So shouldn't all glib-using apps then also be compiled (autoconfigured) with
AC_SYS_LARGEFILE? Is there any way to automagically take care of this?

A simple testprogram which does the following:

  struct stat s;
  stat("a", &s);
  g_stat("b", &s);

does the following on ppc when straced:
stat("a", 0x7ffff880)                   = -1 ENOENT (No such file or directory)
stat64("b", 0x7ffff900)                 = -1 ENOENT (No such file or directory)

but on intel and sparc it does:
stat64("a", 0xeffff6d0)                 = -1 ENOENT (No such file or directory)
stat64("b", 0xeffff7e0)                 = -1 ENOENT (No such file or directory)


So on ppc it's definitely a stat vs. stat64 problem. On sparc there probably is
a different problem from the looks of this :(

Finally found the problem. updateiconcache.c doesn't include <config.h> so the
configured #define _FILE_OFFSET_BITS 64 isn't set when compiling
gtk-update-icon-cache.. Apparently on ppc this causes the usage of both the
normal struct stat and the stat system call, but on sparc it causes the use of
the stat64 call with the normal struct stat (ugh!)

It's still quite nasty that things like this can break other software though
(apparently debian's gaim was also broken because of this type of problem)

Anyway trivial patch attached..

Created attachment 37774 [details] [review]
Add include <config.h> to updateiconcache

The config.h fix is right, but I think we should do #define g_stat stat
(leaving g_stat for bin compat) to avoid problems like this.

Talking about this, isn't there some heavy breakage in defining glib's API/ABI
with something vague such as "stat"?

Isn't it dangerous to allow building glib with or without 64 bits file offsets
with no way for the application to force a call to a stat64 or a stat call?

I think the mere use of "stat" anywhere in the API means there's no cross-host
API, and hence ABI...

Hmm I've been told this could be perceived as irony: it is not.

To clarify: I think that using a "stat" anywhere in an API when stat can mean
different things, such as stat64, is dangerous.

Owen, isn't it risky to redefine g_stat() in the gtk-icon-cache code (and
possibly forget about that later)?  Shouldn't glib be fixed instead?

It feels to me that providing more guarantees about the behaviour of g_stat
would be good, so these problems can't creep up on you like that. Something like
having a G_USE_FILE_OFFSET64 which tells you if it's going to do stat() or
stat64(), or a g_stat struct which always ends up being the right thing, or a
warning at compile-time if you've not got long file support enabled...?

I think Owen meant do #define g_stat stat in gstdio.h for Unix platforms.

Shouldn't for we do that for *all* gstdio functions? There may be issues on some
platforms with mixing open64 et. al with 32-bit io functions.

I've been told too that Owen wants to #define g_stat in the glib headers, not in
the cache source code as I thought.  I agree with Manish that this should be
done for all of glibs 64 bits dependent functions, which will probably require a
lot of work.  :-/

Uh, lot of work? It's just 5 functions + 5 others just for consistency. Not
really a big deal.

Created attachment 37811 [details] [review]
Proposed patch

This patch does the #define aliasing for G_OS_UNIX. It also lets you use the
wrappers if you #define G_STDIO_NO_WRAP_ON_UNIX and maintains ABI.

Tue Feb 22 22:03:38 2005  Manish Singh  <yosh@gimp.org>

        * glib/gstdio.h: On G_OS_UNIX, simple #define g_open and co. as
        aliases for their respective C library functions, instead of
        using the function wrappers. This avoids library users having to
        care about matching large file support with whatever glib has been
        built with. Fixes bug #167942.

        * glib/gstdio.c
        * glib/abicheck.sh
        * glib/glib.symbols
        * glib/makegalias.pl: Logic to make the gstdio wrappers still
        available for compatibility, but not used in new code.