Re: ``struct stat'' issue

[Thomas Schwinge]

Hello!

On Fri, Oct 05, 2007 at 03:29:27PM +0200, I wrote:
> On Sun, Jun 10, 2007 at 11:35:20PM +0200, I wrote:
> > Some months ago I created some patches for the ``struct stat'' issue
> > (<http://savannah.gnu.org/bugs/?18216>).  Perhaps now the time has come
> > for someone to review them?
> 
> It's been some further months now, but I finally sat down and reworked
> the Hurd part of the patch thanks to Samuel's and Roland's comments.

I just installed the changes.


One missing bit is `ext2fs'.  I currectly has code like this:

inode.c
#v+
[...]
  st->st_atime = di->i_atime;
  st->st_mtime = di->i_mtime;
  st->st_ctime = di->i_ctime;

#ifdef XXX
  st->st_atime_usec = di->i_atime.ts_nsec / 1000;
  st->st_mtime_usec = di->i_mtime.ts_nsec / 1000;
  st->st_ctime_usec = di->i_ctime.ts_nsec / 1000;
#endif
[...]
#v-

I.e., the usec / nsec value of variable `st' (``struct stat'') isn't set
at all.  In my reading of the used ``struct ext2_inode'' (variable `di')
there indeed is nothing better than the sec value being provided.  Should
we explicitly set the usec / nsec value of `st' to zero?


> Index: ufs/inode.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/ufs/inode.c,v

> -#ifdef notyet
> -  st->st_atimespec = di->di_atime;
> -  st->st_mtimespec = di->di_mtime;
> -  st->st_ctimespec = di->di_ctime;
> -#else
> -  st->st_atime = read_disk_entry (di->di_atime.tv_sec);
> -  st->st_atime_usec = read_disk_entry (di->di_atime.tv_nsec) / 1000;
> -  st->st_mtime = read_disk_entry (di->di_mtime.tv_sec);
> -  st->st_mtime_usec = read_disk_entry (di->di_mtime.tv_nsec) / 1000;
> -  st->st_ctime = read_disk_entry (di->di_ctime.tv_sec);
> -  st->st_ctime_usec = read_disk_entry (di->di_ctime.tv_nsec) / 1000;
> -#endif

I didn't have a UFS file system to test this (no idea whether the ufs
translator at all still works -- has someone been using it in the last
few years?), but I didn't do the change the ``#ifdef notyet'' way, but
instead like this:

> +  st->st_atim.tv_sec = read_disk_entry (di->di_atime.tv_sec);
> +  st->st_atim.tv_nsec = read_disk_entry (di->di_atime.tv_nsec);
> +  st->st_mtim.tv_sec = read_disk_entry (di->di_mtime.tv_sec);
> +  st->st_mtim.tv_nsec = read_disk_entry (di->di_mtime.tv_nsec);
> +  st->st_ctim.tv_sec = read_disk_entry (di->di_ctime.tv_sec);
> +  st->st_ctim.tv_nsec = read_disk_entry (di->di_ctime.tv_nsec);

I hope that's correct.  I think (if I remember correctly...) that months
ago I had a deeper look at that code and concluded that was indeed the
right way to do this.


> On follow-up: do we want to modify all of the Hurd libraries and servers
> (plus the GNU Mach maptime interface!) to also work on ``struct
> timespec'' (with nanosecond resolution) instead of ``time_value_t'' (with
> microsecond resolution; as it is used at the moment)?

Samuel agreed.  I'll make this a Savannah task item or something.


Regards,
 Thomas

signature.asc
Description: Digital signature