Subject: Re: CVS commit: src/usr.bin/stat
To: Luke Mewburn <lukem@NetBSD.org>
From: Andrew Brown <atatat@atatdot.net>
List: current-users
Date: 07/23/2003 09:17:33
>  | This change however ...
>  | 
>  |   | * remove %B (birth) from default format strings, since %B isn't available
>  |   |   #ifdef HOSTPROG, and it's not standard enough to be fussed about anyway
>  | 
>  | I think is a bug.   Some of those strings (at least) are intended to
>  | dump *everything* (not just everything that someone thinks you might
>  | need to know about).
>
>Well, in that case, the prior behaviour was buggy anyway, because there
>were other stat fields (%f - st_flags - springs to mind) that were NOT
>in those format strings.

you can blame that on me.  i must have forgotten to put it in there.

>  | If you wanted to keep stat as a HOSTPROG (if not, this change can be
>  | undone and forgotten), the solution would be to make %B print some other
>  | time value (st_mtime) most likely) whan stat is compiled as a HOSTPROG.
>
>Or we can just make %B and %f and other "non standard" stuff continue
>to error out in the hostprog version (which is acceptable because we
>know how we are using the hostprog), or print "empty" results for
>unsupport flags.

dyking out the %B stuff works around that particular bit, and makes
stat(1) compile on a 1.6.1 system, but it doesn't do enough towards
making it portable.

(a) stat(1) uses devname(3).  solaris (at the very least) doesn't have
devname(3).  neither does linux.

(b) stat(1) uses strmode(3).  that's easy to cons up, though, so
perhaps it's a bit of a red herring.

(c) stat(1) uses st_mtimespec (and friends), not st_mtime.  it also
expects the timestamps to be timespecs.  on many systems they're not.

(d) stat(1) makes reference to S_IFWHT, which you might not have.  on
the other hand, you might have other things such as S_INSEM, S_INSHD,
S_IFDOOR, or S_IFNAM.

imho, if you don't have st_birthtime on a given system, then stat(1)
should not have any knowledge that there might be such a feature (ie,
all %B handling is ifdef'ed out).  i think it's better to incur an
error if you ask for something you can't get than to silently ignore
the request.  then again, if you're trying to do this portably, 

>  | All of this stuff raises a more general issue as well - when ever a well
>  | defined (but extensible) structure (like struct stat) has something new
>  | added to it, there should also be a "feature test" #define added to the
>  | relevant include file, so other code can test whether this new thing
>  | exists.
>  | 
>  | stat.c has issues with st_flags (which it has a special case #define to
>  | deal with) and now with st_birthtime (etc) for which the HOSTPROG
>  | definition is being used to eliminate.   That's bogus - when st_birthtime
>  | was added, something like
>  | 	#define	_FEAT_ST_BIRTHTIME
>  | should have been added as well, then stat.c could simply do
>  | 
>  | #ifdef _FEAT_ST_BIRTHTIME
>  | #endif
>  | 
>  | and be portable.
>
>Sure that could be done, for all the non-posix st_ fields in stat(1).

true, and then for all the other features and then the features need
versions and and and...isn't this where autoconf gets thrown in?

>However, many people were running into build issues on non-netbsd-current
>platforms, and I made the most appropriate and simplest change that
>the build system required to fix the problems.  If we waited until
>myself or andrew pondered over and the implemented the correct
>solution (a few more days, this is volunteer time), we'd be fielding
>lots more "oh, but i can't build on xxx host platform", and they
>certainly get tedious after a while.

yeah.

-- 
|-----< "CODE WARRIOR" >-----|
codewarrior@daemon.org             * "ah!  i see you have the internet
twofsonet@graffiti.com (Andrew Brown)                that goes *ping*!"
werdna@squooshy.com       * "information is power -- share the wealth."