Subject: Re: CVS commit: src/usr.bin/stat
To: None <lukem@NetBSD.org>
From: Robert Elz <kre@munnari.OZ.AU>
List: current-users
Date: 07/23/2003 17:46:31
    Date:        Wed, 23 Jul 2003 07:23:25 +0000 (UTC)
    From:        Luke Mewburn <lukem@NetBSD.org>
    Message-ID:  <20030723072325.18D44B004@cvs.netbsd.org>

I'm not sure which list is best for this topic ...

  | Modified Files:
  | 	src/usr.bin/stat: stat.c
  | 
  | Log Message:
  | * don't compile in support for st_birth #ifdef HOSTPROG

That will make -current be able to be built again on NetBSD systems
older than (moderately) recent -current (moderately means the past
few months).

However, attempting to make stat into a HOSTPROG is almost certainly
a mistake.   By its very nature, stat is a very NetBSD specific program.
Its purpose is to make available at the shell script level everything
that the stat(2) system call makes available.

Hence the (totally useless, but that's a different issue) st_birthtime,
which made stat.c fail to compile on NetBSD 1.6.1 (or older) or
even on -current from more than a few months ago (before the ufs2
integration) - until your recent fix.

But, stat.c also make available
	st_gen
	st_blocks
	st_blocksize
(and everything else).   How many of those do we really want to
depend upon being available on whatever random system is going to
be used to compile NetBSD ?

Note: none of this is actually necessary (I hope) for the build system,
all it wants stat for is some of the simpler operations.

I'd suggest removing stat from being a HOSTPROG, and instead building
a much simpler, and truly portable program (call it stat, and keep the
syntax compatible if that helps - perhaps even use the stat sources
with HEAVY ifdef) that does just what is needed for the NetBSD build
system's use of it.

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).

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.

The build system most certainly (I hope) isn't going to use %B (nor make
use of its value from any of those default formats), so this would harm
nothing.

All that's needed to do that is to rearrange the order the SHOW_st_?time
cases are handled (and their fall throughs), and have the HOSTPROG
ifdef simply remove the guts of the SHOW_st_btime case, not its label.

That, or #ifdef the format strings as well -- not hard, using

#fidef HOSTPROG
#define PERC_B	"%B "
#else
#define PERC_B ""
#endif

#define STRING "%a %b " PERC_B 	"%c %C ..."


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.

kre