Re: Bug#591386: since: Does not work as expected on GNU/Hurd

[Emilio Pozuelo Monfort <pochu@debian.org>]

Hi,

On 02/08/10 14:10, Axel Beckert wrote:
> "since" does not work as expected on GNU/Hurd (e.g. strauss):
> 
> !57 Z47 ?127 L1 abe@strauss:ttyp0 (-zsh) 11:06:02 [~/debian/since-1.1] > ./since /tmp/since-test
> foo
> !58 Z48 ?0 L1 abe@strauss:ttyp0 (-zsh) 11:06:25 [~/debian/since-1.1] > echo bar >> /tmp/since-test
> !59 Z49 ?0 L1 abe@strauss:ttyp0 (-zsh) 11:06:32 [~/debian/since-1.1] > ./since /tmp/since-test
> foo
> bar
> !60 Z50 ?0 L1 abe@strauss:ttyp0 (-zsh) 11:06:36 [~/debian/since-1.1] > ./since /tmp/since-test
> foo
> bar
> !61 Z51 ?0 L1 abe@strauss:ttyp0 (-zsh) 11:08:07 [~/debian/since-1.1] > ./since /tmp/since-test
> foo
> bar
> !62 Z52 ?0 L1 abe@strauss:ttyp0 (-zsh) 11:08:34 [~/debian/since-1.1] > cat ~/.since
> 00023dea00000003:0000000400000000:0000000400000000
> 00023dea00000003:0000000800000000:0000000400000000
> 00023dea00000003:0000000800000000:0000000400000000
> 00023dea00000003:0000000800000000:0000000400000000
> 
> Except for the data, ~/.since doesn't look different than on my
> laptop. There it also works as it should:

.since is actually wrong. The problem is the format string, it's
%016llx:%016llx:%016llx, but that's not correct on Hurd (due to the size of
dev_t). Adding a printf with the string format not in a variable, so gcc can
check it at compile time and emit warnings, like this:

      printf ("format: %s, 1: %016llx, 2: %016llx, 3: %016llx\n",
              sn->s_fmt, df->d_dev, df->d_ino, df->d_pos);

right before the "result = snprintf() call on line ~1500 emits:

gcc -Wall -O2 -DVERSION=\"1.1\"  -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-DDEBUG -g -o since since.c
since.c: In function ‘update_state_file’:
since.c:1499: warning: format ‘%016llx’ expects type ‘long long unsigned int’,
but argument 3 has type ‘dev_t’

Hardcoding the format string on the "result = snprintf()" call to
"%016x:%016llx:%016llx" makes .since correct:

0000000000000003:000000000003c8ef:0000000000000004

(note the last number is (I guess) the position inside the file, so
0000000400000000 doesn't make sense for a file containing "foo", but 4 does).

Hardcoding the correct format string on the parsing function, since works fine:

root@hurd:~/since/since-1.1$ echo foo > test
root@hurd:~/since/since-1.1$ ./since test
foo
root@hurd:~/since/since-1.1$ ./since test
root@hurd:~/since/since-1.1$ echo bar >> test
root@hurd:~/since/since-1.1$ ./since test
bar
root@hurd:~/since/since-1.1$ ./since test
root@hurd:~/since/since-1.1$ cat /root/.since
0000000000000003:000000000003c8c2:0000000000000008
root@hurd:~/since/since-1.1$

Looking at why the string format is incorrect, I've found that make_fmt_field()
(the function that creates the format string) is called for that argument with a
dev_t argument, while the st_dev member of struct stat is not of type dev_t, but
of __fsid_t. This seems to be a POSIX violation (because sizeof(__fsid_t) !=
sizeof(dev_t), so it would be a bug in Hurd and not in since.

@Other Hurd porters: is my analysis correct?

Regards,
Emilio


P.S.: JFYI the PATH_MAX patch is more a workaround than a fix... the good fix
would be to stop using PATH_MAX and use dynamic allocation instead (in case you
want to fix it properly) though it's not high prio since long filename probably
don't work due to other issues in Hurd.