Re: CVS commit: src/common/lib/libc/string

2020-08-08 Thread David Holland
(yes, change is from January, I'm hugely behind)

On Mon, Jan 27, 2020 at 10:22:03PM +, Andrew Doran wrote:
 > Modified Files:
 >  src/common/lib/libc/string: bcmp.c memcmp.c
 > 
 > Log Message:
 > Drop the alignment check if __NO_STRICT_ALIGNMENT (x86, m68k, vax).

Even on machines that allow unaligned accesses, aligned accesses are
usually faster, so would it be worthwhile to do a few byte comparisons
first until at least one of the arguments is aligned?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc/string

2020-04-04 Thread David Holland
On Thu, Mar 26, 2020 at 10:54:21AM +0700, Robert Elz wrote:
 >   | I don't agree -- because applications shouldn't attempt to modify the
 >   | result, it should be const.
 > 
 > The only reason apps shouldn't modify the string is in case of
 > porting the app to an (well, some) ancient implementation.  Because
 > of the NLS requirements, the message these days (any modern
 > implementation) must be read from some external file - which means
 > the storage for it must be writable.  Nothing else (except the
 > calling thread) cares about the content of the returned message, so
 > there's no actual reason for the app to not modify it now if for
 > some wacky reason it wants to.

I fail to see any scenario in which it's legitimate for an application
to scribble in internal data belonging to libc. Why should this be
permitted?

 > The entry that was in BUGS wasn't a bug - what would be a bug would
 > be if we actually made strerror() return const char * - it wasn't
 > even a limitation (which we traditionally include in BUGS, even
 > though they're generally by design, and not accident, and not
 > really intended to be "fixed") - what it was was a rant about the
 > original design spec, and what's more, one which is no longer
 > warranted.

I don't understand. It is a bug that the library returns a writeable
pointer to data that should not be modified.

Anyway, this is moot because strerror is defined by C; but because it
*should* be const, it should be (and is) declared with __aconst in
string.h.

Please don't change that.

Also, perhaps we should swipe the text about not modifyingthe string
buffer from strsignal.3.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc/string

2020-03-25 Thread David Holland
On Wed, Mar 25, 2020 at 06:50:47PM +, Robert Elz wrote:
 > Modified Files:
 >  src/lib/libc/string: strerror.3
 > 
 > Log Message:
 > Delete the BUGS paragraph about the "missing" const qualifier for the
 > result type of strerror() (and strerror_l()).   While that once should
 > really have been present, when strerror() was invented, there was no
 > "const" qualifier in C to apply, and now the way the code is writtem
 > (really needs to be because of NLS support) the const is no longer
 > really appropriate.
 > 
 > Applications still shouldn't attempt to modify the result however.

I don't agree -- because applications shouldn't attempt to modify the
result, it should be const.

What it points to internally doesn't matter...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/mk

2019-12-20 Thread David Holland
On Thu, Dec 19, 2019 at 11:04:25PM -0500, Christos Zoulas wrote:
 > Module Name: src
 > Committed By:christos
 > Date:Fri Dec 20 04:04:25 UTC 2019
 > 
 > Modified Files:
 >  src/share/mk: bsd.sys.mk sys.mk
 > 
 > Log Message:
 > move MV to sys.mk because it is used there. Pointed out by joerg@

Since the original change was apparently in January, does this need to
be in -9?

-- 
David A. Holland
dholl...@netbsd.org


Re: Leak Sanitizer - how to suppress leaks

2019-09-23 Thread David Holland
On Mon, Sep 16, 2019 at 07:47:24AM +0700, Robert Elz wrote:
 >   | There have been OSes in the past where memory not freed yet at process
 >   | exit is _not_ freed by the system, and there might be again,
 > 
 > Please everyone, let's retain some perspective.   Systems like those
 > (Roy mentioned RTEMS as an example) require specially constructed code,
 > as in a system where process termination doesn't free all the process's
 > resources, then what

The OS I was thinking of was a desktop OS that could (and did) run
quite a bit of unix code. As I recall the various C runtimes available
took some steps to avoid gaping memory leaks, but there's still no
reason to not tidy up when one can.

 > The one reason for doing this kind of free() is so that LSan type analysers
 > can look at memory and report anything that wasn't freed.

This is, however, itself a pretty good reason.

-- 
David A. Holland
dholl...@netbsd.org


Re: Leak Sanitizer - how to suppress leaks

2019-09-15 Thread David Holland
On Fri, Sep 13, 2019 at 07:03:40PM +0700, Robert Elz wrote:
 > It isn't so much that I think we need to save the cost of doing
 > the free() (though for ps it turns out to be harder than you'd expect
 > to actually get it right) but whether it is worth anyone time and
 > effort to actually work out what is needed (if anything at all).  Since
 > ps simply exits, we know there is no real leak, only the illusion of
 > one very briefly.

Keep in mind, though, that one of the roles of NetBSD has always been
to serve as a reference implementation.

There have been OSes in the past where memory not freed yet at process
exit is _not_ freed by the system, and there might be again, and in
cases where it isn't expensive to do so it seems that we may as well
tidy up properly so that the code will run acceptably on such OSes.

In cases where it _is_ expensive, or at least where it's expensive to
figure out, the same argument applies as against garbage collection:
if you aren't sure what the lifetime of that object is, and the
program isn't structured in a way that allows being reasomably sure it
is disposed of exactly once, how can you have confidence in any other
correctness properties?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ufs

2019-02-24 Thread David Holland
On Mon, Feb 25, 2019 at 06:07:23AM +, David Holland wrote:
 > that one doesn't set dropend correctly for small buffers, outsmarted
 > myself while writing it.

Better change (still against 1.242) that makes the logic much
simpler. Will test this overnight...

Index: ufs_vnops.c
===
RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.239
diff -u -p -r1.239 ufs_vnops.c
--- ufs_vnops.c 28 Oct 2017 00:37:13 -  1.239
+++ ufs_vnops.c 25 Feb 2019 06:58:52 -
@@ -1233,7 +1233,7 @@ ufs_readdir(void *v)
 #endif
/* caller's buffer */
struct uio  *calleruio = ap->a_uio;
-   off_t   startoffset, endoffset;
+   off_t   startoffset;
size_t  callerbytes;
off_t   curoffset;
/* dirent production buffer */
@@ -1244,8 +1244,8 @@ ufs_readdir(void *v)
off_t   *cookies;
size_t  numcookies, maxcookies;
/* disk buffer */
-   off_t   physstart, physend;
-   size_t  skipstart, dropend;
+   off_t   physstart;
+   size_t  skipstart;
char*rawbuf;
size_t  rawbufmax, rawbytes;
struct uio  rawuio;
@@ -1256,29 +1256,60 @@ ufs_readdir(void *v)
 
KASSERT(VOP_ISLOCKED(vp));
 
-   /* figure out where we want to read */
+#define UFS_READDIR_ARBITRARY_MAX (1024*1024)
callerbytes = calleruio->uio_resid;
-   startoffset = calleruio->uio_offset;
-   endoffset = startoffset + callerbytes;
-
if (callerbytes < _DIRENT_MINSIZE(dirent)) {
/* no room for even one struct dirent */
return EINVAL;
}
+   if (callerbytes > UFS_READDIR_ARBITRARY_MAX) {
+   callerbytes = UFS_READDIR_ARBITRARY_MAX;
+   }
 
-   /* round start and end down to block boundaries */
+   /*
+* Figure out where to start reading. Round the start down to
+* a block boundary: we need to start at the beginning of a
+* block in order to read the directory correctly.
+*
+* skipstart is the number of bytes we need to read in
+* (because we need to start at the beginning of a block) but
+* not transfer to the user.
+*/
+   startoffset = calleruio->uio_offset;
physstart = startoffset & ~(off_t)(ump->um_dirblksiz - 1);
-   physend = endoffset & ~(off_t)(ump->um_dirblksiz - 1);
skipstart = startoffset - physstart;
-   dropend = endoffset - physend;
 
-   if (callerbytes - dropend < _DIRENT_MINSIZE(rawdp)) {
-   /* no room for even one struct direct */
-   return EINVAL;
-   }
+   /*
+* Now figure out how much to read.
+*
+* callerbytes tells us how much data we can send back to the
+* user. Assume as a starting point that we want to read
+* roughly the same volume of struct directs from disk as the
+* volume of struct dirents we want to send to the user; this
+* is not super accurate for large numbers of small names but
+* will serve well enough. It's ok to underpopulate the user's
+* buffer, and most directories are only a couple blocks
+* anyway.
+*
+* However much we decide we want, stop on a block boundary.
+* That way the copying code below doesn't need to worry about
+* finding partial entries in the transfer buffer.
+*
+* Do this by rounding down (not up) by default as that will
+* with luck avoid needing to scan the next block twice; but
+* always read in at least one block.
+*/
 
-   /* how much to actually read */
-   rawbufmax = callerbytes + skipstart - dropend;
+   /* Base buffer space for CALLERBYTES of new data */
+   rawbufmax = callerbytes + skipstart;
+
+   /* Round down to an integral number of blocks */
+   rawbufmax &= ~(off_t)(ump->um_dirblksiz - 1);
+
+   /* but always at least one block */
+   if (rawbufmax == 0) {
+   rawbufmax = ump->um_dirblksiz;
+   }
 
/* read it */
rawbuf = kmem_alloc(rawbufmax, KM_SLEEP);


-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ufs

2019-02-24 Thread David Holland
On Mon, Feb 25, 2019 at 05:50:08AM +, David Holland wrote:
 > Furthermore, this:
 > 
 >  > +   rawbuf -= dropend;
 > 
 > is entirely wrong (it needs to be "rawbufmax") and without that bound
 > on rawbufmax the code is unsafe...

I repaired this bit just now, so it's not an overt hazard any more.

I still don't like this change all that much but whatever, I suppose...

 > Here's the fix I got bogged down trying to build and test, which also
 > adds a missing upper bound on callerbytes:

that one doesn't set dropend correctly for small buffers, outsmarted
myself while writing it.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ufs

2019-02-24 Thread David Holland
On Sun, Feb 24, 2019 at 07:51:24PM -0500, Christos Zoulas wrote:
 > Module Name: src
 > Committed By:christos
 > Date:Mon Feb 25 00:51:24 UTC 2019
 > 
 > Modified Files:
 >  src/sys/ufs/ufs: ufs_vnops.c
 > 
 > Log Message:
 > drop unused

dropping this logic is wrong...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/netinet

2019-02-24 Thread David Holland
On Sun, Feb 24, 2019 at 05:01:52PM +, Kamil Rytarowski wrote:
 > Modified Files:
 >  src/sys/netinet: sctp_asconf.h
 > 
 > Log Message:
 > Appease GCC7 in sctp_asconf.h
 > 
 > Do not declare types inside function parameter list.
 > Add decklarations of types before these function prototypes.

That warning isn't new... are you sure the observed behavior isn't a
symptom of some other problem elsewhere? :-/

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/bin/sleep

2019-01-28 Thread David Holland
On Mon, Jan 28, 2019 at 07:45:12PM +0700, Robert Elz wrote:
 >   | at least the Arabic ones (momayyez). Supporting 2-3+ styles is opening
 >   | Pandora's box;
 > 
 > No-one is planning that.   There are two options - the C locale, or
 > the locale set in the environment.

Actually it's kind of what I was suggesting -- that shell programs
that take floats should accept any string with an unambiguous parse in
any representation style, and reject ones that do not.

Although in some ways this:

 > (unless we someday make a command that works for LC_NUMERIC
 > the way iconv works for LC_CTYPE).

is a better plan.

Either way you want to be able to combine inputs that have different
representations of the same data type, because in the real world
people have colleagues from all over (just note the Cc: list on this
email); they will from time to time send to you material using their
local conventions, and you want to be able to process it without
making a fuss or cursing them afterwards.

The assumption of locales is that every user's data world is
geographically small, and that wasn't true even when they were
invented.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2019-01-27 Thread David Holland
On Sun, Jan 27, 2019 at 07:10:24PM +0100, Maxime Villard wrote:
 > > Restore satlink's majors entries commented out and marked obsolete.
 > > Otherwise they might accidentally get reused later and cause a
 > > security problem.
 > 
 > This is completely useless, please revert. You are re-adding references
 > to satlink without a good reason.

There's a good reason right there in the commit message, y'know. No,
I'm not going to revert it.

 > We've never marked entries as obsolete in majors (just check amd64's
 > cvs log). If you really think this matters, then please add all the
 > obsoletes that are needed after all these years of changes.

Yes, that's done now. It became clear that it was necessary, but it
took a while.

(Your recent removals were more than half the work.)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/bin/sleep

2019-01-27 Thread David Holland
On Sat, Jan 26, 2019 at 12:28:08PM +0100, Kamil Rytarowski wrote:
 > This is where I disagree. In my opinion (of a native user of ",") -
 > parsing locale specific input for such programs doesn't make sense.
 > 
 > Locale specific format is in my opinion appropriate only for programs
 > that process text for printing (like man(1) or groff(1)).

So here's the thing: there's an underlying design problem here that
nobody (not us, not POSIX, and certainly not anyone in the Linux
world) has really worked out a solution to, and that is:

The Unix shell environment is about processing text, and, largely,
processing text in arbitrary ad hoc ways. It fundamentally relies on
being able to treat the user-facing output of arbitrary programs as
machine-readable input. This puts the goal of customizing user-facing
output to accomodate the user in direct conflict with the goal of
making the shell environment work as intended.

One 'solution' is to discourage ordinary users from learning the shell
environment ("...for the console is dark, and full of terrors") so
that it can be arbitrarily broken (via ill-considered locale behavior
and other things) without repercussions. That seems to be the Linux
world's approach. That won't work for NetBSD, if only because there
are too many oldtimers here. Another 'solution' is to create a
separate but equal set of additional tools for use when the output
needs to be predictable. This seems to be what POSIX favors, but it's
not right either: it compromises the design, since the point, or part
of the point, has always been that the shell allows you to paste
together the same things that you use directly. Also it multiplies
entities needlessly.

Some other approach is needed. It's not particularly clear what.


-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/etc

2019-01-13 Thread David Holland
On Mon, Jan 14, 2019 at 09:42:54AM +1100, matthew green wrote:
 > it would be OK if this was _read-only_ access to network
 > configuration, but one should never be allowed to change the
 > it unless root.

In the long run, it's quite helpful for laptops to be able to adjust
the network configuration from a GUI on the console without having to
run GUI bits as root. We aren't in a position to do this correctly
(nor does importing the likes of polkit as a hack to allow reasoning
about being "on the console" constitute correctly) but let's not lose
track of it as a goal.

Is there a way we could, for example, leverage the current hacks for
chowning console devices to grant access to wpa_supplicant?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.sbin/sysinst

2018-11-04 Thread David Holland
On Sat, Nov 03, 2018 at 06:30:00PM +, Martin Husemann wrote:
 > Remove "usage" translations - they never could be displayed as we only select
 > the language later.

What happens in the scenario when someone's using sysinst after
installation and has a locale set?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/make

2018-04-05 Thread David Holland
On Wed, Apr 04, 2018 at 08:31:11PM -0400, Christos Zoulas wrote:
 > Module Name: src
 > Committed By:christos
 > Date:Thu Apr  5 00:31:11 UTC 2018
 > 
 > Modified Files:
 >  src/usr.bin/make: parse.c
 > 
 > Log Message:
 > Be more selective about detecting a SYSV include as opposed to a dependency
 > line. Dependency lines should contain a '::' operator or ':'.

This is wrong: it's perfectly legal to write "foo.o:foo.c".

It needs to scan for variables, or at least not look inside matching
sets of () {}.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/man/man9

2017-12-08 Thread David Holland
On Fri, Dec 08, 2017 at 03:52:01PM +, Taylor R Campbell wrote:
 > Modified Files:
 >  src/share/man/man9: mutex.9
 > 
 > Log Message:
 > Specify memory ordering implied by mutex_(spin_)enter/exit.
 > 
 > I'm hesitant to just say `implies membar_enter/exit' -- that may be a
 > little stronger than we intend, since we don't really mean to
 > guarantee anything about loads and stores before the mutex_enter or
 > after the mutex_exit.  But we probably end up implementing the
 > semantics that we imply membar_enter/exit on all CPUs.

The definition of membar_enter and membar_exit is the way it is to
make the stores involved in *doing* the enter and exit work. This
should not really be exposed outside the mutex code... on some
processors it's possible to order those stores with the ones "inside"
without affecting anyone else.

So I would say that the problem here is in the way membar_enter/exit
are defined; or rather, that they're called that (which is confusing
anyway) rather than e.g. membar_store_any() and membar_any_store().

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2017-12-07 Thread David Holland
On Mon, Dec 04, 2017 at 07:41:56PM +0100, Maxime Villard wrote:
 > > Are you saying Christos is lying about it working after reverting your
 > > commits?
 > 
 > Why don't you read the previous emails from this thread, instead of
 > coming up with random, inaccurate statements like this one? I said
 > very clearly that COMPAT_20 for example does not compile. Same for
 > COMPAT_16 and COMPAT_10, actually.

Of course I read the thread. The thing is, all of those compiled fine
for me, at least after reverting your recent vandalism.

And also for Christos.

This does raise a few questions.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/etc/rc.d

2017-12-06 Thread David Holland
On Wed, Dec 06, 2017 at 08:25:08AM +0700, Robert Elz wrote:
 > It isn't the precedence of the operators that is at issue, but
 > deciding what is an operator -

oh right, I'm pretty sure I knew about that at one point and blocked
it out for the sake of my sanity :-)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/etc/rc.d

2017-12-05 Thread David Holland
On Mon, Dec 04, 2017 at 02:50:33PM +, Robert Elz wrote:
 > Modified Files:
 >  src/etc/rc.d: sshd
 > 
 > Log Message:
 > Do away with (not well specified, even if it happens to work) absurd
 > 15 arg test ([ ]) expression, and replace it with several well defined
 > 2 arg tests, combined with (also well defined) sh syntax.

Test -o isn't well specified? Or is the issue the precedence of ! vs. -o?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch/amd64/conf

2017-12-03 Thread David Holland
On Sat, Dec 02, 2017 at 09:59:02AM +, Maxime Villard wrote:
 > Module Name: src
 > Committed By:maxv
 > Date:Sat Dec  2 09:59:02 UTC 2017
 > 
 > Modified Files:
 >  src/sys/arch/amd64/conf: ALL
 > 
 > Log Message:
 > Remove options that do not exist on amd64.

Compile-testing those is still useful.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/dev/acpi/wmi

2017-12-03 Thread David Holland
On Sun, Dec 03, 2017 at 11:53:37PM +0100, Manuel Bouyer wrote:
 > On Sun, Dec 03, 2017 at 11:06:47AM -0800, bch wrote:
 > > ...
 > > /usr/src/sys/dev/acpi/wmi/wmi_dell.c: In function 'wmi_dell_action':
 > > /usr/src/sys/dev/acpi/wmi/wmi_dell.c:234:16: error: comparison between
 > > signed and unsigned integer expressions [-Werror=sign-compare]
 > >   for (i = 0; i < __arraycount(wmi_dell_actions); i++) {
 > > ^
 > > cc1: all warnings being treated as errors
 > 
 > that's strange, I don't get it on my sources tree.

Odd that you wouldn't, but maybe it depends on whether you're building
a kernel or something rumpity.

 > Would changing i to unsigned int fix the warning ?

Yes. Provided of course that it doesn't break elsewhere that way :-)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2017-12-03 Thread David Holland
On Sun, Dec 03, 2017 at 10:05:08AM +0100, Maxime Villard wrote:
 > Le 02/12/2017 ? 22:23, David Holland a ?crit :
 > > On Sat, Dec 02, 2017 at 10:04:26PM +0100, Maxime Villard wrote:
 > >   > > Revert this. Compat on amd64 must be available all the way back to
 > >   > > 0.9, same as i386.
 > >   > >
 > >   > > Also, please stop unilaterally breaking the world.
 > >   >
 > >   > You are kidding, right? Everything below COMPAT_15 has *never* been
 > >   > enabled.  This change does not break anything, since nothing was
 > >   > enabled in the first.
 > > 
 > > No, I am not kidding. It is there in GENERIC so it can be enabled for
 > > people who want to run very old i386 binaries.
 > 
 > Good, and apparently no one has noticed that some options don't compile.

Are you saying Christos is lying about it working after reverting your
commits?

 > > what authority do you
 > > think you have to make declarations about what will and won't be
 > > removed?
 > 
 > As I said earlier, this "please ask core for approval" argument
 > does not work on me anymore, sorry about that. So I'm going to
 > commit right away now, on whatever directly has to do with amd64.

"sorry about that" is not the right answer. Like it or not, getting
rulings from core is the way this project resolves controversial
technical points when group consensus fails. Having to do it a lot is
not a positive thing: it means you aren't working with the rest of us.

In fact, you don't seem to even attempt to gather a consensus before
making changes like this, and now you're rejecting the project's
fallback decision structure as well. You're acting unilaterally and
brushing aside all objections.

This is really not how things can go; if you don't change your
attitude, you're going to end up getting kicked off the project. That
would be unnecessary and unfortunate, but you aren't offering much in
the way of alternatives.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src

2017-12-02 Thread David Holland
On Wed, Oct 04, 2017 at 07:54:33PM -0400, Christos Zoulas wrote:
 > Log Message:
 > Add NOBINARIES, useful to build tools are libraries which is what's needed
 > for mknative.

Wouldn't it make more sense for this to be a build target than a
setting (as in, say, "build.sh libs")?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2017-12-02 Thread David Holland
On Sat, Dec 02, 2017 at 10:04:26PM +0100, Maxime Villard wrote:
 > > Revert this. Compat on amd64 must be available all the way back to
 > > 0.9, same as i386.
 > > 
 > > Also, please stop unilaterally breaking the world.
 > 
 > You are kidding, right? Everything below COMPAT_15 has *never* been
 > enabled.  This change does not break anything, since nothing was
 > enabled in the first.

No, I am not kidding. It is there in GENERIC so it can be enabled for
people who want to run very old i386 binaries.

 > "Compat on amd64 must be available"
 > 
 > What authority do you have to say that? It has never been this way.

Providing compat has been policy for 25+ years.

Since you bring up the notion of authority... what authority do you
think you have to make declarations about what will and won't be
removed?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2017-12-02 Thread David Holland
On Sat, Dec 02, 2017 at 01:03:15PM +, Maxime Villard wrote:
 > Modified Files:
 >  src/sys/arch/amd64/conf: GENERIC files.amd64
 >  src/sys/arch/xen/conf: files.xen
 > Removed Files:
 >  src/sys/arch/amd64/amd64: compat_13_machdep.c
 > 
 > Log Message:
 > Drop COMPAT_13 on amd64, already not enabled. Reduces the number of
 > critical places.

Revert this. Compat on amd64 must be available all the way back to
0.9, same as i386.

Also, please stop unilaterally breaking the world. This is getting to
be a regular occurrence and that is not the way things are supposed to
be done in this project.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch/x86/x86

2017-10-03 Thread David Holland
On Tue, Oct 03, 2017 at 06:54:52PM +0200, Maxime Villard wrote:
 > I'm not responding to this nonsensical thread anymore,

The problem is, you keep acting unilaterally without having gathered a
consensus, then ignoring the resulting objections and demanding to
have everything your way and only your way.

Your ideas about how to proceed (about this or any of the other recent
issues) are not the only possible correct ways forward.

(Obviously, mine are.)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/misc

2017-03-25 Thread David Holland
On Fri, Mar 24, 2017 at 12:49:43PM +, Brian Ginsbach wrote:
 > Modified Files:
 >  src/share/misc: acronyms.comp
 > 
 > Log Message:
 > Add the more prevalent media access control (MAC) in addition to
 > medium access control

It doesn't really need both...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/fs/tmpfs

2017-01-11 Thread David Holland
On Wed, Jan 11, 2017 at 12:12:33PM +, Joerg Sonnenberger wrote:
 > Modified Files:
 >  src/sys/fs/tmpfs: tmpfs_vnops.c
 > 
 > Log Message:
 > Remove RO check in tmpfs_putpages for now, the syncer doesn't like the
 > error code.

Either removing it is wrong or it should be changed to KASSERT :-)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys

2017-01-03 Thread David Holland
On Wed, Jan 04, 2017 at 09:17:23AM +0800, Paul Goyette wrote:
 > > > /* info for a single history event */
 > > > struct sysctl_history_event {
 > > > -struct timespec she_tspec;
 > > > +time_t  she_time_sec;
 > > > +uint32_tshe_time_usec;
 > > > +uint32_tshe_filler;
 > > 
 > > I would make those explicitly sized types, uint64_t x 2
 > 
 > Sure!

also make it nsec...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/doc

2017-01-02 Thread David Holland
On Mon, Jan 02, 2017 at 05:18:49AM +0100, Kamil Rytarowski wrote:
 > > Historically exect() is used by debuggers to start debuggees. While
 > > it's equivalent to using PT_TRACE_ME followed by execve(), I think the
 > > result is that the new process first stops immediately after the exec
 > > finishes so that the debugger doesn't have to worry about stepping
 > > through the exec call in its own code.
 > > 
 > > This doesn't mean it shouldn't go away (or as much away as it can,
 > > that is, to COMPAT_70) but I'm not convinced there's no case for it.
 > > 
 > 
 > So, can I change exect(3) to something like:
 > 
 > int
 > exect(const char *path, char *const argv[], char *const envp[])
 > {
 > if (ptrace(PT_TRACE_ME, 0, NULL, 0) == -1)
 > return -1;
 > return execve(path, argv, envp);
 > }
 > 
 > The current implementation of exect(3) (at least philosophically)
 > predates SIGTRAP on exec().

Not really; AFAIK, with that the target will first stop at the return
from the ptrace call, and also stop at the exec, whereas with
traditional exect it will first stop after the exec completes, in
__start or in ld.elf_so.

An unsophisticated debugger expecting the latter will almost certainly
be confused by the former.

Anyway, it needs to be kept in both the kernel and libc for compat so
there's not much to be gained by mucking with it. :-/

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/doc

2017-01-01 Thread David Holland
On Sat, Dec 31, 2016 at 08:57:16PM +, Kamil Rytarowski wrote:
 > Update TODO.ptrace
 > 
 > Mark exect(3) for removal, there is no use-case for it. exec() is already
 > monitored and emits SIGTRAP when traced.

Historically exect() is used by debuggers to start debuggees. While
it's equivalent to using PT_TRACE_ME followed by execve(), I think the
result is that the new process first stops immediately after the exec
finishes so that the debugger doesn't have to worry about stepping
through the exec call in its own code.

This doesn't mean it shouldn't go away (or as much away as it can,
that is, to COMPAT_70) but I'm not convinced there's no case for it.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libm/complex

2016-12-31 Thread David Holland
On Sat, Dec 31, 2016 at 08:33:04PM +, co...@sdf.org wrote:
 > On Sat, Dec 31, 2016 at 03:33:03PM +, Maya Rashish wrote:
 > > -  w = r + y * I;
 > > +  r = sqrt(x);
 > > +  w = r;
 > >}
 > >}
 > >return w;
 > 
 > I'm an alignment newbie.
 > Could this create alignment issues?
 > Return type is double complex, and r is just double.

It's not a pointer, so it's the compiler's problem.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/kern

2016-12-28 Thread David Holland
On Wed, Dec 28, 2016 at 07:32:05PM +, Taylor R Campbell wrote:
 > It's true that moving the kauth call expanded the attack surface a
 > little bit.  Now we have to worry about:

When I saw the original commit I wondered it if it was an information
leak. Maybe it's not, but that's certainly a fourth consideration.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc/net

2016-12-06 Thread David Holland
On Tue, Dec 06, 2016 at 01:41:03PM -0500, Christos Zoulas wrote:
 > Module Name: src
 > Committed By:christos
 > Date:Tue Dec  6 18:41:03 UTC 2016
 > 
 > Modified Files:
 >  src/lib/libc/net: linkaddr.c
 > 
 > Log Message:
 > Fix buffer copy without checking the size of input:
 > https://www.kb.cert.org/vuls/id/548487

Doesn't this need 

-   if (out >= obuf + sizeof(obuf)) \
+   if (out >= obuf + sizeof(obuf) - 1) \

to avoid having the null terminator off the end?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch/amd64/conf

2016-11-26 Thread David Holland
On Sat, Nov 26, 2016 at 09:21:17PM +1100, matthew green wrote:
 > > Modified Files:
 > >src/sys/arch/amd64/conf: GENERIC
 > > 
 > > Log Message:
 > > 1.6 was the first amd64 release. (although it didn't really work too well
 > > before -5)
 > 
 > dunno what you're talking about.  i ran it for many years before -5.

If you like :-) I'm just going by what I saw in gnats at the time.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2016-11-25 Thread David Holland
On Fri, Nov 25, 2016 at 01:04:20PM +0100, Maxime Villard wrote:
 > ? It probably means there is some kind of stupid assumption or hack in
 > emacs. If you could send me the core, the binary and tell me which arch it
 > is, that would be nice. 

No, if there is a problem it is somewhere in crtstuff, ld.elf_so, or
early libc initialization. When it dies, it dies long before main and
long before anything in emacs itself gets control.

In particular, _libc_init chokes on a wrong __ps_strings value
apparently either passed in from the kernel, garbaged by ld.elf_so, or
maybe otherwise mishandled somewhere in the startup logic.

 > AFAICT, emacs is known to be buggy on netbsd.

Rubbish.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch

2016-11-25 Thread David Holland
On Thu, Nov 24, 2016 at 10:28:56PM +0900, Masanobu SAITOH wrote:
 > > Put a one-page redzone between userland and the PTE space on amd64 and
 > > i386.
 > > 
 > > The PTE space is a critical region that maps the page tree, and bugs have
 > > been found in both amd64 and i386 where the kernel would wrongly overflow
 > > userland data on this area. This kind of bug is terrible, since it allows
 > > userland to overwrite some entries of the page tree, which makes it easy
 > > to patch the kernel text and get ring0 privileges.
 > 
 > My emacs dumps core with change.
 > 
 > What should we do?

Make maxv clean up his mess, or rebuild emacs. It seems that the stack
location gets baked in somehow when emacs dumps, although I don't
really see exactly how (see PR 51654) and this causes existing emacs
binaries to stop working.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/dev/usb

2016-10-30 Thread David Holland
On Mon, Oct 31, 2016 at 04:15:23AM +, NONAKA Kimihiro wrote:
 > Modified Files:
 >  src/sys/dev/usb: usbdevs
 > 
 > Log Message:
 > Remove duplicated HUAWEI E353 entry.

Woops, sorry about that.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sbin/mount

2016-10-14 Thread David Holland
On Sun, Oct 09, 2016 at 12:57:50PM -0600, Greg Oster wrote:
 > > Modified Files:
 > >src/sbin/mount: mount.c
 > > 
 > > Log Message:
 > > change warning message
 > > 
 > > previously attempting to use mount -t ext2 like myself would result in
 > > the warning "mount: mount_ext2 not found for /mnt", which (if you're
 > > me) can be misunderstood as "/mnt is not an ext2 filesystem"...
 > > 
 > > change it to "mount: mount_ext2 not found"
 > 
 > Consider running the following:
 > [snip]

That's why the conventional error message format goes like

   mount: /mnt4: mount_ext2 not found

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/dev

2016-10-14 Thread David Holland
On Thu, Oct 13, 2016 at 08:56:31AM +, Ryo Shimizu wrote:
 > Modified Files:
 >  src/sys/dev: mm.c
 > 
 > Log Message:
 > /dev/mem cannot lseek over 0x8000 on 32bit architectures.

This isn't enough (at least pedantically) because uio_offset is
signed.

(in fact, since uio_offset is signed, I imagine seeking to/past
0x8000    on 64-bit architectures also has problems)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/kern

2016-10-11 Thread David Holland
On Mon, Oct 10, 2016 at 10:23:46AM +0300, Valery Ushakov wrote:
 > > Log Message:
 > > PR 49636 Henning Petersen: use "0L" to return 0 from a function returning
 > > long, and test its returned value against "0L" instead of "0".
 > > 
 > > This is not especially necessary, but it's also harmless.
 > 
 > No, it's not.  It sends a strong signal that integer promotions are
 > not enough here, and that explicit cast/sufix is required for some
 > reason.  Yet, no such reason is provided in the PR.  Please revert.
 > Or are you going to tag all long zeroes in the tree as 0L?

Why pursue the whole tree? "A foolish consistency..."

anyway, like I said last night, if you care that much feel free to
revert it.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/fs/nfs

2016-09-23 Thread David Holland
On Thu, Sep 22, 2016 at 11:57:06AM -0400, Christos Zoulas wrote:
 > Modified Files:
 >  src/sys/fs/nfs: files.newnfs
 > 
 > Log Message:
 > add missing attribute

I didn't think that even configured yet...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/dev/ic

2016-09-18 Thread David Holland
On Sun, Sep 18, 2016 at 09:52:37PM +, Jaromir Dolecek wrote:
 > Modified Files:
 >  src/sys/dev/ic: ld_nvme.c
 > 
 > Log Message:
 > must use PR_NOWAIT also during ldattach()/dkwedge discover, our i/o is there
 > called with a spin lock held, which triggers LOCKDEBUG panic

That sounds like it should be corrected upstream of nvme...?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/distrib/sets/lists/modules

2016-09-17 Thread David Holland
On Sun, Sep 18, 2016 at 06:28:12AM +0800, Paul Goyette wrote:
 > > > Modified Files:
 > > >  src/distrib/sets/lists/modules: md.amd64 md.i386 mi
 > > >
 > > > Log Message:
 > > > Fix sets lists for nvme module.  Since it is being built only for the
 > > > i386 and amd64 platforms, the entries belong in the md.xxx lists, not
 > > > in the mi list.
 > > 
 > > Shouldn't it be built for all platforms that are capable of plugging
 > > one in?
 > 
 > Probably.  but Jaromir only set it up for i386/amd64, and I was just
 > following his lead.

Fair enough. So, who wants to be the first to connect an nvme to their
vax? :-)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/distrib/sets/lists/modules

2016-09-17 Thread David Holland
On Sat, Sep 17, 2016 at 02:27:19AM +, Paul Goyette wrote:
 > Modified Files:
 >  src/distrib/sets/lists/modules: md.amd64 md.i386 mi
 > 
 > Log Message:
 > Fix sets lists for nvme module.  Since it is being built only for the
 > i386 and amd64 platforms, the entries belong in the md.xxx lists, not
 > in the mi list.

Shouldn't it be built for all platforms that are capable of plugging
one in?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ext2fs

2016-08-24 Thread David Holland
On Tue, Aug 23, 2016 at 02:24:30AM -0400, Christos Zoulas wrote:
 > Modified Files:
 >  src/sys/ufs/ext2fs: ext2fs_vfsops.c
 > 
 > Log Message:
 > CID 1371644: use strlcpy, remove dup copy.

You sure about the dup copy? Those are different string buffers.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/xlint/lint1

2016-07-20 Thread David Holland
On Wed, Jul 20, 2016 at 02:14:12PM -0400, Christos Zoulas wrote:
 > Modified Files:
 >  src/usr.bin/xlint/lint1: cgram.y
 > 
 > Log Message:
 > accept attributes in param decls

Parsing gcc attributes "correctly" (as in, where gcc accepts them) is
a trainwreck. Do we really want to bother trying?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc_fp

2016-07-15 Thread David Holland
On Thu, Jul 14, 2016 at 01:59:19AM +, Matt Thomas wrote:
 > Added Files:
 >  src/lib/libc_fp: Makefile gcc-softfloat.c
 >  src/lib/libc_fp/arch/mips: Makefile.inc fpdf.S fpsf.S shlib_version
 > 
 > Log Message:
 > Library which implements the softfloat primitives using FP instructions
 > Currently contains only untested MIPS routines.
 > XXX move arm libc_vfp to here.
 > 
 > [...]
 > #ifdef MIPS3
 > #defineCOP1_SYNC   nop
 > #else
 > #defineCOP1_SYNC
 > #endif

Can you call this MTC1_HAZARD like the MFC0_HAZARD that got added
recently elsewhere? It's a pipeline hazard and not a synchronization
(which to me at least indicates some form of handshake) and it's also
specific to mtc1/dmtc1.

(also while this code appears to currently be 64-bit only, in the long
run the condition is wrong...)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/arch/x86/x86

2016-07-11 Thread David Holland
On Mon, Jul 11, 2016 at 09:42:20AM +, Kengo NAKAHARA wrote:
 > Log Message:
 > strncpy should use destination buf length instead of source buf length.
 > 
 > pointed out by nonaka@n.o.

this should almost certainly be strlcpy, not strncpy.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/sys

2016-07-06 Thread David Holland
(from a while back)

On Sun, Jan 24, 2016 at 02:11:56PM +0100, Joerg Sonnenberger wrote:
 > On Fri, Jan 22, 2016 at 11:31:30PM +, David A. Holland wrote:
 > > Module Name:   src
 > > Committed By:  dholland
 > > Date:  Fri Jan 22 23:31:30 UTC 2016
 > > 
 > > Modified Files:
 > >src/sys/sys: dirent.h
 > > 
 > > Log Message:
 > > Uses __GNUC_PREREQ__, needs sys/cdefs.h.
 > 
 > I wonder if including stddef.h / libkern.h and using the real offsetof
 > would be better.

Wouldn't help; the GNUC_PREREQ is there because it uses typeof. Which
it needs to do, because the macro in question needs to be polymorphic
over struct dirent and also struct direct from FFS. *That* is what
ought to be tidied up.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/kern

2016-06-27 Thread David Holland
On Mon, Jun 20, 2016 at 03:14:35PM -0400, Christos Zoulas wrote:
 > Modified Files:
 >  src/sys/kern: kern_exec.c
 > 
 > Log Message:
 > put back commented out name resolution code that was gc'ed after previous
 > refactoring.

Wait a moment, I thought we'd put in code to do getcwd for that case?
Or did it get taken out again?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/dev/pci

2016-06-19 Thread David Holland
On Sun, Jun 19, 2016 at 06:58:17AM +, David A. Holland wrote:
 > Modified Files:
 >  src/sys/dev/pci: arcmsr.c
 > 
 > Log Message:
 > Broaden the #if NBIO > 0 block. Should fix broken emips build.

Erm: "evbppc"

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys

2016-06-05 Thread David Holland
On Sun, Jun 05, 2016 at 01:33:04PM +, Maxime Villard wrote:
 > Modified Files:
 >  src/sys/arch/i386/stand/lib: bootmenu.c menuutils.c
 >  src/sys/lib/libsa: gets.c stand.h
 > 
 > Log Message:
 > Use gets_s instead of gets. The x86 bootloader prompt is easy to
 > overflow.

That's not gets_s; how about a different name?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/sys

2016-03-21 Thread David Holland
On Mon, Mar 21, 2016 at 01:36:22AM +0100, Joerg Sonnenberger wrote:
 > Here is a trivial test case showing that the basic problem exists for
 > both clang and gcc:
 > 
 >int a, b
 > 
 >int f(void) {
 >  return  != 
 >}

Do you perhaps mean "extern int a, b;"? That's an important
distinction. In particular I can't find anything in C99 that
guarantees that two such declarations for which storage is not
reserved can't refer to the same object.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/tests/lib/libutil

2015-12-31 Thread David Holland
On Thu, Dec 31, 2015 at 04:14:52PM -0500, Greg Troxel wrote:
 > > When evaluated on a Sunday, "next Sunday" means 7 days in the future,
 > > not 14. When evaluated on a Monday, it apparently means 13 days in the
 > > future. There's not exactly a spec for parsedate.y, so conform to the
 > > implementation.
 > >
 > > PR 50574.
 > >
 > > XXX: to me at least this is an odd notion of "next Sunday", but
 > > whatever...
 > 
 > It's clearly a bug.
 > 
 > On a Monday, "next Sunday" is 6 days away.  "Sunday week" is 13.
 > 
 > So perhaps the test should be correct, and the implementation fixed.

Well, it treats "this Sunday" as 6 days away. It apparently thinks
"this X" is in the next seven days and "next X" is the week past that.

That is not entirely crazy, though it's not really consistent with the
conventional usage I know... but that usage isn't well defined. On
Monday, "this Sunday" and "next Sunday" are pretty clearly the same
day; but on Saturday, they probably aren't, and it's not clear what
the crossover is.

Meanwhile, these usages are regional. I've never understood the "next
Sunday week" or "this Sunday week" business no matter how many times I
run across it in lit from across the pond.

Anyhow, if you care, I suggest writing a spec for parsedate }:-)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: [netbsd-5] src/sys/ufs/lfs

2015-11-09 Thread David Holland
On Mon, Nov 09, 2015 at 09:59:18AM +, Stephen Borrill wrote:
 > Modified Files:
 >  src/sys/ufs/lfs [netbsd-5]: lfs_segment.c
 > 
 > Log Message:
 > Fix breakage from ticket #1974
 > 
 > 
 > To generate a diff of this commit:
 > cvs rdiff -u -r1.213.8.1 -r1.213.8.2 src/sys/ufs/lfs/lfs_segment.c

crap, sorry! I forwarded this fix to pullup-6 and I guess I forgot
about -5.

and, um, this needs to go in 5-1 and 5-2 since the wrong change went
into those too.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/conf

2015-09-03 Thread David Holland
On Thu, Sep 03, 2015 at 02:56:35PM +0900, Masao Uebayashi wrote:
 > Because ${OBJS} affects not only build order but also link order.

And this matters why, in the absence of e.g. profile-driven function
layout?

(although it might be interesting to see if running ${OBJS} through
lorder/tsort improves the output kernel)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/conf

2015-09-02 Thread David Holland
On Thu, Sep 03, 2015 at 01:30:18AM +, Masao Uebayashi wrote:
 > Module Name: src
 > Committed By:uebayasi
 > Date:Thu Sep  3 01:30:18 UTC 2015
 > 
 > Modified Files:
 >  src/sys/conf: Makefile.kern.inc
 > 
 > Log Message:
 > Stop ordering objects alphabetically now that I am sure I can fix fallouts.

Why change this? It is useful to be able to estimate how far along
your build is by what's being built.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys

2015-08-29 Thread David Holland
On Sat, Aug 29, 2015 at 03:51:53PM +, Masao Uebayashi wrote:
  According to nxr.netbsd.org, nothing uses MEMORY_DISK_IMAGE.  Retire it.
  Premature design and its build rule bloats Makefile.kern.inc.  There are
  other ways like MEMORY_DISK_DYNAMIC.
  
  (When kernel will be built as relocatable, embedding binary data will be much
  easier, and md(4), splash(4), and ksyms(4) will all benefit.)

Um, this kind of thing should be discussed.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sbin/fsck_lfs

2015-08-23 Thread David Holland
On Sun, Aug 23, 2015 at 12:00:23PM -0400, Christos Zoulas wrote:
  Module Name: src
  Committed By:christos
  Date:Sun Aug 23 16:00:23 UTC 2015
  
  Modified Files:
   src/sbin/fsck_lfs: pass0.c
  
  Log Message:
  swap the formats too, not just the args.

sorry, I shouldn't commit when that tired.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/external/mit/xorg

2015-08-10 Thread David Holland
On Sun, Aug 09, 2015 at 01:06:30AM +0200, Aymeric Vincent wrote:
  David Holland dholland-sourcechan...@netbsd.org writes:
  
 Fix bracket expressions by moving '-' to the end of them. GNU awk 
   choked.
  
   Front is safer. fwiw.
  
  OK, I moved them to the front, together with '_' because it felt awkward
  to separate the separators. Out of curiosity, is it safer just because
  it is more robust to future additions to the expression or is it
  actually safer even if the expression is left as is?

It's safer in the sense that a broken regexp parser is unlikely to
accidentally treat the first character of a character set as the
middle - in a range; but if the last character is - it might interpret
the second-last character, the -, and closing ] as a range, with
unfortunate results. It is not very likely; but stranger things have
happened, and it doesn't help that regexp tools have a long-standing
culture of not complaining about invalid regexp syntax.

anyway it's a very minor point.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/external/mit/xorg

2015-08-08 Thread David Holland
On Sat, Aug 08, 2015 at 10:27:00PM +, Aymeric Vincent wrote:
  Modified Files:
   src/external/mit/xorg: xorg-pkg-ver.mk
  
  Log Message:
  Fix bracket expressions by moving '-' to the end of them. GNU awk choked.

Front is safer. fwiw.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/games/atc

2015-07-27 Thread David Holland
On Sun, Jul 26, 2015 at 10:22:24PM +, Thomas Klausner wrote:
  Modified Files:
   src/games/atc: atc.6
  
  Log Message:
  Use An in AUTHORS section.

Shouldn't the uucp address be .Aq'd too?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/lfs

2015-07-25 Thread David Holland
On Sat, Jul 25, 2015 at 10:40:35AM +, Martin Husemann wrote:
  Modified Files:
   src/sys/ufs/lfs: lfs_bio.c lfs_debug.c lfs_pages.c lfs_segment.c
   lfs_vnops.c
  
  Log Message:
  Use accessors in DEBUG and DIAGNOSTIC code as well

Woops, sorry...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc/arch

2015-07-15 Thread David Holland
On Wed, Jul 15, 2015 at 02:23:40PM +, Antti Kantee wrote:
  Modified Files:
   src/lib/libc/arch/i386/gen: Makefile.inc
   src/lib/libc/arch/x86_64/gen: Makefile.inc
  
  Log Message:
  Remove objects built from C sources comments.  Everyone can see
  they're built from C sources because the source files end in .c (???)

That comment matches the comment about objects built from assembler
sources 12-13 lines up. I would put it back...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.sbin/rtadvd

2015-07-03 Thread David Holland
On Fri, Jun 05, 2015 at 03:41:59PM +, Roy Marples wrote:
  Module Name: src
  Committed By:roy
  Date:Fri Jun  5 15:41:59 UTC 2015
  
  Modified Files:
   src/usr.sbin/rtadvd: advcap.c if.c rrenum.c rtadvd.c
  
  Log Message:
  Use %m in syslog(3) instead of %s strerror(3).

Was this really necessary? Last I remember we considered %m a legacy
mistake and preferred to avoid it.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.sbin/rtadvd

2015-07-03 Thread David Holland
On Fri, Jul 03, 2015 at 08:12:42PM +0100, Roy Marples wrote:
   Modified Files:
 src/usr.sbin/rtadvd: advcap.c if.c rrenum.c rtadvd.c
   
   Log Message:
   Use %m in syslog(3) instead of %s strerror(3).
   
   Was this really necessary? Last I remember we considered %m a legacy
   mistake and preferred to avoid it.
  
  It made the program smaller, which is a good thing.
  There is no mention of syslog %m being considered a legacy mistake
  in the man page. Maybe you have a link to any discussion of this?

I'm not sure I'm going to be able to find it readily. Searching for %m
doesn't work real well, even with grep.

however, here's what it's about. %m is a wart: since it's not part of
printf, it means you can't use printf to implement syslog(3), at least
not without adding a bunch of gross hacks. Or alternatively (like the
gnu folks do) you can quietly add %m support to printf, which is
pretty gross as well.

There is no real reason %m is necessary and we were at least
discussing deprecating it; I don't think anything formal came of that
but still it's probably better not to add new uses of it.

anyone else remember? Christos?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/miscfs/specfs

2015-06-29 Thread David Holland
On Mon, Jun 29, 2015 at 12:25:49PM -0400, Christos Zoulas wrote:
  Module Name: src
  Committed By:christos
  Date:Mon Jun 29 16:25:49 UTC 2015
  
  Modified Files:
   src/sys/miscfs/specfs: spec_vnops.c
  
  Log Message:
  Revert previous, and explain why.

Hrm, shouldn't it be vp?

(as it is, it's using the uvm_object lock pointer as the key)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/compat/netbsd32

2015-06-23 Thread David Holland
On Tue, Jun 23, 2015 at 06:23:36PM +1000, matthew green wrote:
   Log Message:
   Don't reference netbsd32_nfssvc unless NFSSERVER is defined.
   Fixes PR 49994.
  
  thanks.

*bow*

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/utoppya

2015-06-21 Thread David Holland
On Sat, Jun 20, 2015 at 04:42:32PM +, Hubert Feyrer wrote:
  Seriously: give the reader of the manpage an idea on what this program
  is for without forcing them to Google or read another manpage.

Thank you!

(there are quite a few man pages that would benefit from this treatment...)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.sbin/rpcbind

2015-05-09 Thread David Holland
On Sat, May 09, 2015 at 05:22:18PM -0400, Christos Zoulas wrote:
  Modified Files:
   src/usr.sbin/rpcbind: rpcbind.c
  
  Log Message:
  use EXIT_SUCCESS/EXIT_FAILURE consistently.
  
  
  To generate a diff of this commit:
  cvs rdiff -u -r1.21 -r1.22 src/usr.sbin/rpcbind/rpcbind.c

heh, I just stuck EXIT_FAILURE in the one I added because it was new.

that said,

-   exit(2);
+   exit(EXIT_FAILURE);

I wonder if anything depended on that being 2 and not 1.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/misc

2015-04-27 Thread David Holland
On Sun, Apr 26, 2015 at 09:24:26PM +0200, Marc Balmer wrote:
   If you persist in this attitude, you're likely to generate a consensus
   within this part of the Internet culture to document your sudden
   lack of a commit bit.
  
  David, I think this a uneeded threat.  rodent's commit might not be
  appropriate in some communities, but they do not violate the contract a
  developer has with TNF.

That wasn't a threat, that was an observation.

Remember, regardless of the content there's a clear set of
expectations and procedures for what to do when people object to your
commits. Ignoring the (widespread) objections and blazing away with
more of the same is not part of it. Neither is passive-aggressive
name-calling.

  I personally find these acronyms neither very
  sizzling nor very insulting.  They just exist as they are and listing
  them I think is fine.  We are solely the collectors of acronyms,
  including them does not mean NetBSD endorses the use of them, I think.
  There are probably more offensive (but good) jokes in fortune -o...

You're welcome to your point of view. But keep in mind that it's
likely coloured by not having much personal experience with threats of
sexual violence.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/misc

2015-04-26 Thread David Holland
On Sun, Apr 26, 2015 at 06:09:37PM +0900, Masao Uebayashi wrote:
  Are you removing any inappropriate/hostile words in the tree (if exists)?

If there's stuff of the same sort as the acronyms that got pruned,
perhaps so. Don't know of any, but that doesn't mean it's not there.

(routine bad language, not so much; for that it's better to fix the
code to not need swearing at)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/misc

2015-04-25 Thread David Holland
On Fri, Apr 24, 2015 at 12:20:17AM +, Blue Rats wrote:
  Module Name: src
  Committed By:rodent
  Date:Fri Apr 24 00:20:17 UTC 2015
  
  Modified Files:
   src/share/misc: acronyms
  
  Log Message:
  +BB = baby - with this commit, we document, rather than police, yet another
  acronym/abbreviation which is part of Internet culture. Those who feel
  otherwise are welcome to turn this over to core@, who resolve disputes
  among developers.

If you persist in this attitude, you're likely to generate a consensus
within this part of the Internet culture to document your sudden
lack of a commit bit.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ffs

2015-02-24 Thread David Holland
On Mon, Feb 23, 2015 at 01:38:54PM +, Maxime Villard wrote:
  Modified Files:
   src/sys/ufs/ffs: ffs_vfsops.c
  
  Log Message:
  Small changes:
   - instead of always calling DPRINTF with __func__, put __func__ directly
 in the macro
   - ffs_mountfs(): rename fsblockloc - fs_sblockloc, initialize fs_sbsize
 to zero
  No real functional change

Those two really should have been committed separately though.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ffs

2015-02-14 Thread David Holland
On Sat, Feb 14, 2015 at 08:07:39AM +, Maxime Villard wrote:
  Modified Files:
   src/sys/ufs/ffs: ffs_appleufs.c
  
  Log Message:
  ffs_appleufs_validate():
   - remove superfluous printfs
   - ensure ul_namelen!=0, otherwise the kernel accesses ul_name[-1] and
 overwrites the previous field in the structure.

Did you test this? It is almost certain that this bit:

*n = *o;
-   n-ul_checksum = 0;
n-ul_checksum = ffs_appleufs_cksum(n);

breaks it. Also, I think you might want to keep the print when the
checksum is wrong.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/rsh

2014-10-30 Thread David Holland
On Wed, Oct 29, 2014 at 11:54:41PM -0700, John Nemeth wrote:
  On Oct 30,  6:13am, David A. Holland wrote:
  } 
  } Module Name:   src
  } Committed By:  dholland
  } Date:  Thu Oct 30 06:13:50 UTC 2014
  } 
  } Modified Files:
  }src/usr.bin/rsh: rsh.c
  } 
  } Log Message:
  } Drop setuid before execing rlogin. Failure to do so should be
  } harmless, but is sloppy.
  
   Uh...
  
  -r-xr-xr-x  1 root  wheel  16303 Sep 18 17:35 /usr/bin/rsh*

-r-sr-xr-x  1 root  wheel  16169 Sep 11 04:45 /bin/rcmd

It doesn't *work* if not setuid.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/rsh

2014-10-30 Thread David Holland
On Thu, Oct 30, 2014 at 04:40:57PM +, David Holland wrote:
} 
} Module Name:src
} Committed By:   dholland
} Date:   Thu Oct 30 06:13:50 UTC 2014
} 
} Modified Files:
} src/usr.bin/rsh: rsh.c
} 
} Log Message:
} Drop setuid before execing rlogin. Failure to do so should be
} harmless, but is sloppy.

 Uh...

-r-xr-xr-x  1 root  wheel  16303 Sep 18 17:35 /usr/bin/rsh*
  
  -r-sr-xr-x  1 root  wheel  16169 Sep 11 04:45 /bin/rcmd
  
  It doesn't *work* if not setuid.

Although I suppose that code is outside IN_RCMD. So maybe it's
useless; but on the other hand, what are the odds of someone taking
the code and installing it the traditional way? Plus I'm sure the
Coverity report that triggered this discussion in the first place
thought the code was running setuid.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/config

2014-10-30 Thread David Holland
On Thu, Oct 30, 2014 at 09:27:06PM +0900, Masao Uebayashi wrote:
  So, while you expect that options works before it's defined, you
  also expect the order is honored for no use.  I'm not sure how it
  can work internally.
  
  At this moment, no are evaluated when it's parsed.  Those no agp*
  fallouts happened because agp is re-selected while resolving
  dependency after all parsing is done.  IMO anything relying on order
  tends to be broken by design.  For example: if BAR depends on FOO, no
  options FOO has to disable BAR too, because BAR can't be enabled
  without FOO.  But when you re-enable FOO, BAR is not enabled.  Is this
  really what you're expecting?

I think it's important not to break the semantics of this.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/conf

2014-09-26 Thread David Holland
On Fri, Sep 19, 2014 at 09:05:23AM +, Matt Thomas wrote:
  Added Files:
   src/sys/conf: filesystems.config
  
  Log Message:
  include for configs which includes all file-systems and any dependent
  pseudo-devices

Was this discussed anywhere?

I'm not saying it's a bad idea; I'm just not convinced this is how we
want to do it.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc

2014-09-26 Thread David Holland
On Thu, Sep 25, 2014 at 03:08:29PM +, Emmanuel Dreyfus wrote:
  Modified Files:
   src/lib/libc/include: namespace.h
   src/lib/libc/sys: Makefile.inc
  Added Files:
   src/lib/libc/sys: fdiscard.c posix_fallocate.c
  
  Log Message:
  Fix argument paddiing for posix_fallocate and fdiscard with gcc 1.x

gcc *1.x*?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/external/bsd/nvi/docs/USD.doc/vi.ref

2014-09-11 Thread David Holland
On Wed, Sep 10, 2014 at 05:44:22PM -0400, Christos Zoulas wrote:
  Modified Files:
   src/external/bsd/nvi/docs/USD.doc/vi.ref: Makefile ref.so vi.ref
  
  Log Message:
  Fix the index building which got completely broken after the last update.

I thought I'd tested it, but maybe I failed to merge something. Or I'm
thinking of a different doc with crazy index building.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/external/bsd/nvi/docs/USD.doc/vi.ref

2014-09-11 Thread David Holland
On Thu, Sep 11, 2014 at 09:20:54PM +, Christos Zoulas wrote:
Modified Files:
 src/external/bsd/nvi/docs/USD.doc/vi.ref: Makefile ref.so vi.ref

Log Message:
Fix the index building which got completely broken after the last update.
  
  I thought I'd tested it, but maybe I failed to merge something. Or I'm
  thinking of a different doc with crazy index building.
  
  Well, you probably did not test it with OBJDIR...

Well... I'd have thought that the missing ${.CURDIR}s in the makefile
wouldn't work with any objdir setting. (Except none, which doesn't
count.) But it must have worked somehow at some point... that lot got
merged across when nvi got moved to external, and I could have fumbled
something at that point, but I did build the tree before committing,
and lots of people have done builds since.

Also, I thought I'd beaten it hard enough to not need .sy any more,
but I must be thinking of some other crazy autogenerated index. Or
maybe there was a whole changeset on this one that I fumbled...

Bleh.

This fix should probably go into -7...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/usr.bin/make

2014-09-09 Thread David Holland
On Tue, Sep 09, 2014 at 02:19:02PM +0200, Joerg Sonnenberger wrote:
   Log Message:
   Restore apb's 20140820 commit (-r1.228 of main.c):
   It should not be an error to have VAR != command that prints no output
   
   Joerg reverted a bit too enthusiastically.
  
  Thanks. I will go over the various changes after Tuesday if noone beats
  me to it. I am aware that there are some non-problematic ones in the
  patch as well as subtile issues with others.

Yeah, that's what I was getting started on.

There's a lot of changes. There's three main changesets, of which the
big one was over 5000 lines of diff; and the changes include things
like braces changes as well as substance so it's a pain to review in
detail.

I've been thinking I should create a secondary repo for this with a
branch for each of the changesets, so they can be handled
incrementally. If you're interested in sharing such a thing, let me
know.

-- 
David A. Holland
dholl...@netbsd.org


Re: C++ keyword/scope/c. changes

2014-09-08 Thread David Holland
On Mon, Sep 08, 2014 at 03:02:42PM +, Taylor R Campbell wrote:
  Why the sudden spate of changes in the kernel to change keywords, move
  struct and enum definitions, insert pointless casts, c.?  Was this
  discussed anywhere?

Beats me, and not that I saw, respectively.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libperfuse

2014-09-08 Thread David Holland
On Fri, Sep 05, 2014 at 01:57:19PM +, Emmanuel Dreyfus wrote:
 Modified Files:
  src/lib/libperfuse: ops.c perfuse.c
 
 Log Message:
 Improve POSIX compliance of FUSE filesystems through PERUSE
 - access denied is EPERM and not EACCES
   
   wait, what?
  
  EACCES is when you lack permission for the parent directories. 
  This errno is enforced by the LOOKUP method. If you have access
  to the directory but not to the object itself, you get EPERM,
  and this is enforced by the SETATTR method, which will not be
  called if you did not succeed LOOKUP first.
  [...]

If you're sure that all the cases match ffs (which we believe to be
correct on this stuff, modulo any kauth glitches) then I have no
objection. It's just that most fs-level permission failures are
supposed to be EACCES so I was alarmed.

  Please Cc: me for the reply because I am not subscribed to this 
  list. And your reply is needed even if you agree because I filled
  a pullup ticket (I did not see you reply), which is now stalled 
  because of your objection: I now need your approval :-)

Ticket number?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/common/lib/libprop

2014-09-06 Thread David Holland
On Fri, Sep 05, 2014 at 05:19:25AM +, Matt Thomas wrote:
  Modified Files:
   src/common/lib/libprop: prop_ingest.c prop_number.c
  
  Log Message:
  Eliminate use of C++ keywords and don't nest struct definitions.

Why do we care if proppropliblib compiles in a C++ compiler? It is C
code.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libperfuse

2014-09-03 Thread David Holland
On Wed, Sep 03, 2014 at 04:01:45PM +, Emmanuel Dreyfus wrote:
  Modified Files:
   src/lib/libperfuse: ops.c perfuse.c
  
  Log Message:
  Improve POSIX compliance of FUSE filesystems through PERUSE
  - access denied is EPERM and not EACCES

wait, what?

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src

2014-07-29 Thread David Holland
On Tue, Jul 29, 2014 at 05:53:59AM -0400, Greg Troxel wrote:
   Also, note in the ucom(4) man page that there is about 1 ms of
   latency.  Discussed on tech-kern in October of 2013, with the only
   concern being that someone who didn't know what they were doing might
   set up a stratum 1 server, and that somehow might have worse
   timekeeping than whatever else that person might have done; the man
   page comment is a mitigation for this.
  
   latency of 1ms and the same jitter - only ucycom uses interrupt
   endpoints for data transfer
  
  I was abstracting on purpose, because no other man page describes
  performance characteristics in such detail.  (When I tested, my best
  guess was that the delay from PPS edge capture ranged from 400us to
  1400us as the USB frame clock shifted.)

In few other cases does it matter at this level either...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/external/gpl3

2014-07-27 Thread David Holland
On Sun, Jul 27, 2014 at 09:23:26AM +0200, Martin Husemann wrote:
   hmmm, we should try to get the URL working again.  it's baked into
   old binaries we will support for a while yet...
  
  The redirects seem to work already, this change should be backed out.

Why? Even if it works, it's still an outdated url.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/share/man/man9

2014-07-25 Thread David Holland
On Fri, Jul 25, 2014 at 08:38:29AM +, Thomas Klausner wrote:
  Modified Files:
   src/share/man/man9: vnodeops.9
  
  Log Message:
  New sentence, new line. Punctuation formatting nits.

Woops, sorry about that.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/kern

2014-07-25 Thread David Holland
On Fri, Jul 25, 2014 at 11:21:47AM +0200, Maxime Villard wrote:
   Log Message:
   Add fdiscard and posix_fallocate syscalls.
  
  I think 'error' instead of 'result' is better; it makes clear that it's
  an error code, and it's consistent with the other functions in the file.

!

If you care that much, change it...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src

2014-07-20 Thread David Holland
On Sat, Jul 19, 2014 at 10:23:31PM +0200, Marc Balmer wrote:
   That's not how we update 3rd-party code in the tree. We use cvs import
   and resolve conflicts afterwards.
  
  Oh well..  Having recent Lua is more important than following a
  procedure that is difficult to follow.

*boggle*

Really, it's not difficult.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ffs

2014-07-11 Thread David Holland
On Thu, Jul 10, 2014 at 11:15:54AM -0400, Christos Zoulas wrote:
  Modified Files:
   src/sys/ufs/ffs: ffs_wapbl.c
  
  Log Message:
  CID 975226: hande error from UFS_WAPBL_BEGIN

That won't work; AFAICT, if that UFS_WAPBL_BEGIN fails you need to do
fs-fs_flags |= FS_DOWAPBL before calling ffs_wapbl_stop or it'll
assert.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/ufs/ffs

2014-07-11 Thread David Holland
On Fri, Jul 11, 2014 at 12:17:29PM -0400, Christos Zoulas wrote:
  Modified Files:
   src/sys/ufs/ffs: ffs_wapbl.c
  
  Log Message:
  move the flag setting higher to avoid KASSERT (dholland)

unfortunately... I don't think that's right either. I'm not sure what
might happen if it sets the flag before beginning the transaction...

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys

2014-07-01 Thread David Holland
On Tue, Jul 01, 2014 at 10:16:02AM +, Ryota Ozaki wrote:
  Log Message:
  Lock IFQ operations when NET_MPSAFE
  
  - Introduce NET_MPSAFE
- not defined by default

If it compiles (and is expected to continue to compile) please add it
to ALL.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc

2014-06-24 Thread David Holland
On Mon, Jun 23, 2014 at 07:46:15PM +, Taylor R Campbell wrote:
 instead? (And can a reference to this be stuffed into the man page?)
  
  Read from /dev/urandom.

...ugh. Can we provide a wrapper around this for transparent casual
use? (Even if it's in libutil and marked not for general consumption?)
Having to open-code the logic every time is really not conducive to
doing things right, and in most arbitrary programs linking with some
crypto lib to start up specific alternative stream ciphers (which will
likely go out of date before the next time someone touches the code
again) would be entirely inappropriate.

There is a reason they came up with and deployed arc4random(), even
though hardwiring it by name to a particular cipher was silly.

  Or it may be worthwhile to mostly keep the way arc4random(3) works but
  replace the PRNG, as in the first reimplementation of arc4random(3)
  above, but rename it.

yes please

  With either of the later two cases, perhaps we
  ought to just coopt random(3) for the purpose.

no please (random(3) is not expected to be cryptographically strong,
so code that assumes it is becomes unportable in a subtle and
dangerous way)

Also I think there's code out there that saves and restores the
random(3) state and expects to get repeatable results.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/lib/libc

2014-06-23 Thread David Holland
On Mon, Jun 23, 2014 at 02:14:10PM +, Taylor R Campbell wrote:
  Modified Files:
   src/lib/libc: shlib_version
  
  Log Message:
  Add `remove arc4random' to mythical libc major bump todo list.

I'm not saying I disagree, but what's new code supposed to use
instead? (And can a reference to this be stuffed into the man page?)

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/external/bsd/cron/dist

2014-06-22 Thread David Holland
On Sat, Jun 21, 2014 at 02:20:59PM +, Christos Zoulas wrote:
  Modified Files:
  src/external/bsd/cron/dist: do_command.c
  
  Log Message:
  Log some actual information on various failures, so you can tell
  what happened.
  
  ??? This code is not used. We are using the LOGIN_CAP code. What are you
  trying to fix?

Isn't it? (All of it?) I had a problem at one point (a few months
back, I think, it took me a while to remember to fetch the patch out
and commit it) that was printing a useless message, and I added some
extra information to diagnose it, and in the course of found a lot of
similar diagnostics and patched them all for completeness.

-- 
David A. Holland
dholl...@netbsd.org


  1   2   3   4   5   >