Re: CVS commit: src/sys/arch

2021-01-25 Thread Robert Elz
Date:Mon, 25 Jan 2021 08:19:44 -0800
From:Jason Thorpe 
Message-ID:  

  | Using { 0 } makes an assumption about the first member of the
  | structure which is not guaranteed to remain true.

That's right, but you could explicitly init a named field, most likely
the one that is tested to determine that this is the sentinel, eg: from
one of the recent updates ...

 static const struct device_compatible_entry compat_data[] = {
{ .compat = "pnpPNP,401" },
-   { 0 }
+   { }
 };

that could instead be changed to
{ .compat = NULL }

(or something similar to that).

kre



Re: CVS commit: src/external/bsd/unbound/lib/libunbound

2021-01-01 Thread Robert Elz
Date:Fri, 1 Jan 2021 20:38:36 +
From:"Roy Marples" 
Message-ID:  <20210101203836.2cadcf...@cvs.netbsd.org>

  | Modified Files:
  | src/external/bsd/unbound/lib/libunbound: Makefile
  |
  | Log Message:
  | libunbound: Now we use libevent, don't build mini_event or winsock_event.

That changes the ABI of  the library (libunbound) and so at the very
least would require a major version bump.

It also breaks the build.

Please revert, and also (and this applies to *everyone*) please do release
builds BEFORE committing changes which can (even remotely possibly) affect
the rest of the build (which more or less means any change other than to
doc - and even there if any doc is new or deleted).

kre



Re: please put back cat man pages, and what's the deal with warp?

2020-11-10 Thread Robert Elz
Date:Tue, 10 Nov 2020 10:07:32 +0100
From:Kamil Rytarowski 
Message-ID:  

  | Revert MKCATPAGES change?

No, the changes to the mtree config that stopped creating the catN
directories, and anything else that has happened that is not 100% MKCATPAGES.

On games/dungeon

  | but it needs rewrite (from Fortrant) and to avoid (c) issues.

A rewrite form Fortran will not avoid copyright issues, that's
just a translation, and translations are covered.

kre



Re: CVS commit: src/sbin/mount

2020-10-24 Thread Robert Elz
Date:Sat, 24 Oct 2020 10:51:34 +
From:"Nia Alarie" 
Message-ID:  <20201024105134.6539bf...@cvs.netbsd.org>

  | file systems that are used as what spools?

Ah, youth!

The old text:
This option is useful for optimizing read performance on file systems
that are used as news spools.
was refering to netnews (usenet, NNTP or whatever).

Such a filesystem is almost certainly not going to care about access times
(this is about uses for the noatime flag, to save others needing to go look
at what changed) as the news articles are referenced by large numbers of
news readers, and outgoing feeds, and when any of that happened is of no
interest to the server or its maintainers, at all - but it happens (or
happened, when usenet was a big thing) a lot, and not bothering to update
the access times could mean a lot of unnecessary writes to an already very
busy filesystem.

All that said, note that I am not objecting to the change (referring to
flash based filesystems rather than news spools) it is a little more
currently relevant.

kre



Re: CVS commit: src/external/gpl3/gcc/dist/gcc/config/aarch64

2020-10-16 Thread Robert Elz
Date:Fri, 16 Oct 2020 04:07:31 +
From:"Thomas Mueller" 
Message-ID:  <20201016052422.e063084...@mail.netbsd.org>

  | Should I add ,linux to the end of the procfs line?

You can, but it isn't needed these days -- I used to mount procfs twice,
once without the linux option, on /proc, and once with, on /emul/linux/proc)
but there seems to be little point in that any more (even though the linux
/proc has a whole bunch of trash that has nothing to do with processes, and
should be, and generally is, available from /kern ... /proc/cpuinfo is an
example of that, though that one is missing from kernfs and should be added
there).

I do add "hidden" to the mount option list though, there's essentially
no point in including /proc /kern /dev/pts (or anything else like those)
in default df output (which is the only thing "hidden" generally affects).

kre



Re: CVS commit: src/external/public-domain/tz/dist

2020-10-08 Thread Robert Elz
Date:Thu, 08 Oct 2020 19:11:59 +1100
From:matthew green 
Message-ID:  <22915.1602144...@splode.eterna.com.au>

  | at least pacificnew is referenced by the build still:

Yes, sorry, the way that tzdata updates get done makes it
essentially impossible to test what is going to happen until
after it is done (I suspect that's true for most of the
things we handle using cvs import).   One of the things that
cvs doesn't make easy.

I fixed it when my test build failed because of it, and then
Nick fixed the remaining build problem just about the same time
my restarted build found that one (thanks).

kre



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

2020-09-14 Thread Robert Elz
Date:Mon, 14 Sep 2020 16:16:52 +
From:"Roland Illig" 
Message-ID:  <20200914161652.d4eb5f...@cvs.netbsd.org>

  | make(1): inline LPAREN in parse.c
  |
  | It's shorter and more readable, and the other characters don't have
  | named constants as well.

Most likely the reason for that was for parentheses matching editors.

Using '(' creates a ( that (a non C syntax savvy) editor will match
against the next (otherwise unmatched) ')' it finds.   LPAREN doesn't
have that effect.   An alternative is to add a /*)*/ comment on the line,
but that starts getting obtrusive, and difficult to justify.

kre



Re: CVS commit: src/bin/kill

2020-08-30 Thread Robert Elz
Date:Sun, 30 Aug 2020 16:10:40 +
From:"Robert Elz" 
Message-ID:  <20200830161040.57630f...@cvs.netbsd.org>

  | Log Message:
  | Use the POSIX specified format [...]

Ugh ... that should have mentioned that this applies to the
output of "kill -l"

kre



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

2020-07-11 Thread Robert Elz
Date:Sat, 11 Jul 2020 18:24:51 +0300
From:Kimmo Suominen 
Message-ID:  <20200711152451.ga1...@homeworld.netbsd.org>

  | On Sat, Jul 11, 2020 at 05:00:02PM +0200, Martin Husemann wrote:
  | > I don't understand the change. When was this broken? This has always 
worked
  | > for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7.
  |
  | I think all real SCSI hardware I've had has always just only had LUN 0,
  | and each disk has been on its own SCSI ID (target).

Just to make things clear here, the LUN you're talking about is not
the scsi unit number (which is what I think Martin was referring to)
but a sub-device number within a single scsi ID.   Right?

In real scsi hardware, the only place I think I've ever seen other than
LUN 0 is in a raid array device, where there is a single scsi bus
attachment, with a single ID, and then each raid volume created gets
a different LUN (not all scsi raids work that way I think, but some do).

kre



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

2020-06-29 Thread Robert Elz
Date:Sun, 28 Jun 2020 22:58:52 +0300
From:Valery Ushakov 
Message-ID:  <20200628195852.gc20...@pony.stderr.spb.ru>

  | but I'd expect people that actually use IFS with octal digits
  | or a backslash to also understand and know how to add necessary
  | quoting in that case.

You'd think so, right, but ...   and that assumes that the person
writing the printf code (copying the example from the man page) knows
what IFS is, or that it might have been altered.

  | Though perhaps it makes sense to them back for
  | pedagogical purposes.

That, and because it simply is the right way to code in sh 99% of the
time - omitting needed quotes is a major source of bizarre bugs.   When
they're like this, and only trigger in unusual circumstances, they're ever
harder to find.

kre



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

2020-06-28 Thread Robert Elz
Date:Fri, 26 Jun 2020 22:05:05 +
From:"Valeriy E. Ushakov" 
Message-ID:  <20200626220505.e9030f...@cvs.netbsd.org>

  | Modified Files:
  | src/usr.bin/printf: printf.1
  |
  | Log Message:
  | Drop redundant quoting in the nested printf example.

I don't think that is correct, the quotes around 0x0A were obviously
redundant (and changing it to 0x0a doesn't alter that), but those
around the $(printf ...) are not - without those the result from the
command substitution would be subject to field splitting, and if IFS
happens to contain a \ or an octal digit, bad things will happen.

In general it is never a good idea to omit quotes around sh word
expansions except in the cases where you actually want field splitting
(or pathname expansion, etc) to happen (and sometimes except in the
contexts where those don't occur anyway, like x=$whatever)

For the example in question, try running it with IFS=1 or IFS=2
to see what I mean.

kre



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

2020-06-28 Thread Robert Elz
Date:Sat, 27 Jun 2020 11:49:30 -0400
From:"Christos Zoulas" 
Message-ID:  <20200627154930.84e22f...@cvs.netbsd.org>

  | Modified Files:
  | src/sys/compat/sys: mount.h
  |
  | Log Message:
  | Ignore the supplied size, and always use the argument size that we know.

Is this fix correct?Certainly looks more reasonable than
what was there before, as the supplied size (for no seemingly
good reason) is often 0, but in compat_20_sys_getfsstat() (in
sys/compat/common/vfs_syscalls_20.c) statvfs_to_statfs12_copy()
is called, via do_sys_getvfsstat() with a size of
sizeof(struct statvfs90) and a statvfs90 (compat/sys/statvfs.h)
is certainly not the same size as a statfs12 (compat/sys/mount.h)

Or perhaps (probably more likely) compat_20_sys_getfsstat() should
be passing sizeof(struct statfs12) instead ?   (And now may as well
just pass 0).

kre



Re: CVS commit: src

2020-06-04 Thread Robert Elz
Date:Fri, 5 Jun 2020 04:19:09 +0200
From:Kamil Rytarowski 
Message-ID:  <99440f2e-c0fc-5e47-4f8b-137bdf5a3...@netbsd.org>


  | I can see the problem now. It's a fault in ksh(1).

Whether this actually amounts to being called a "fault" or not is
not so clear ... most systems don't name the RT signals, that appears
to be a BSD affectation.

Even on NetBSD, when using bosh ()available in pkgsrc) the results are ...

bosh $ kill -l
HUP INT QUITILL TRAPABRTEMT FPE KILLBUS
SEGVSYS PIPEALRMTERMURG STOPTSTPCONTCHLD
TTINTTOUIO  XCPUXFSZVTALRM  PROFWINCH   INFOUSR1
USR2PWR RTMIN   RTMIN+1 RTMIN+2 RTMIN+3 RTMIN+4 RTMIN+5 RTMIN+6 RTMIN+7
RTMIN+8 RTMIN+9 RTMIN+10RTMIN+11RTMIN+12RTMIN+13   
RTMIN+14 RTMIN+15RTMAX-14RTMAX-13
RTMAX-12RTMAX-11RTMAX-10RTMAX-9 RTMAX-8 RTMAX-7 
RTMAX-6RTMAX-5  RTMAX-4 RTMAX-3
RTMAX-2 RTMAX-1 RTMAX
bosh $ 

(ignoring the funky formatting, made worse by my cut & paste).

In POSIX, just SIGRTMIN and SIGRTMAX are defined (and "kill" is
required to drop the SIG part) so bosh just invents "names" based
upon those two.

kre

ps: in modern NetBSD, you might want to try /bin/sh as your interactive
shell, it does most of what ksh can do (which is useful interactively, there's
no "select" command, but I can't imagine that being of interactive use ..
or actually of any real use).   If there's some particular impediment,
something lacking that you feel is required (something ksh can do which sh
cannot) let me know, and if it seems reasonable, it could be added.




Re: CVS commit: src

2020-06-04 Thread Robert Elz
Date:Fri, 5 Jun 2020 01:50:47 +0200
From:Kamil Rytarowski 
Message-ID:  

  | What happened to RT signal names?
  |
  | I'm not sure what's wrong as this code works under a debugger.

No idea right now, but I will take a look.

Which shell are you using for that?   And which version.   That's
relevant as kill is almost always a built in, so when you just
run "kill -l" you're getting whatever the shell knows, but when you
run it under the debugger, you're getting /bin/kill (you could also
try running that explicitly, see if it makes a difference).

kre



Re: CVS commit: src/tests/lib/libc/sys

2020-05-11 Thread Robert Elz
Date:Mon, 11 May 2020 13:47:45 +0200
From:Kamil Rytarowski 
Message-ID:  <54178983-82d1-df3d-fd54-549a6c73f...@gmx.com>

  | The only purpose of the test is to check whether misaligned program
  | counter can crash the kernel (it can for NetBSD/sparc). Later, if a
  | process dies or runs is not important, thus it is being killed.

That's all fine.

  | A process can disappear after dying and before reappearing as a zombie.

There's a state between running and dead (zombie), that's correct - but
it really doesn't matter, once the process ceases to be alive, it is beyond
killing any more.

  | This is not a bug, but a predicted race.

Yes, that's what I said, and that's fine too.

  | Doing the kill once (and missing the process) is still possibly enough,
  | but correcting it with SIGKILL does not cost.

No, there is no problem with doing the SIGKILL, but if fails for the
above reason, there's absolutely no point trying again.  The process is
gone, it isn't coming back.   It doesn't need killing.

But even if there was a reason to try again (there isn't), one more
attempt would do - inserting an infinite loop is folly.

But I see that you have fixed it, that's good, what's there now looks
much better.   Thanks.

kre




Re: CVS commit: src/tests/lib/libc/sys

2020-05-11 Thread Robert Elz
Date:Mon, 11 May 2020 11:03:15 +
From:"Kamil Rytarowski" 
Message-ID:  <2020050315.54b13f...@cvs.netbsd.org>

  | Do not fail when trying to kill a dying process
  |
  | A dying process can disappear for a while. Rather than aborting, retry
  | sending SIGKILL to it.

I don't understand this ... a process should never be able to
disappear and then reappear (not in any way).   If a SIGKILL (or
ptrace(PT_KILL) fails with a "no such process" error, then repeating
it won't (or shouldn't) help - if it does, there's a kernel bug that
needs fixing (and it is OK for the test to fail until that happens.)

Further, if the reason for this failure is that the process is
dying, you probably never needed the kill in the first place (and
no, I don't mean it should be deleted - the parent is unlikely to
know the state of the child, so killing it, if that is what is needed
is the right thing to do ... just that if the kill fails because you
were too late issuing it, it isn't an error, just a race that you lost,
and certainly shouldn't be repeated).

But more than that, adding an infinite loop to the test, where you keep
doing the kill forever until it succeeds, or errno somehow stops being
ESRCH looks like a recipe for disaster.

Just do the kill once, ignore the error if it is ESRCH (and probably
also ECHILD) report other errors as failures.

kre



Re: CVS commit: src/bin/kill

2020-05-06 Thread Robert Elz
Date:Wed, 6 May 2020 11:28:42 +0200
From:Kamil Rytarowski 
Message-ID:  


  | While there, we have got a long standing issue with wait.1 man page,

Huh!   I had no idea any such thing existed...  (do you know of any
more bizarre ones like that?)

  | it should be either removed (as the wait(1) program is gone) or adapted
  | with the reality of being a builtin.

Yes, it should.Was there ever a wait(1) program?   POSIX says there
should be (along with cd umask ulimit ...) but the general feeling here
is that that's just plain stupid...   (and I agree).


I'm not going to right now, as I'm not sure which is the right path
to take - there has been (in the past) some discussion about making
man pages for all of the sh builtins (so one doesn't need to know the
trick of how to find their doc in the sh(1) manpage easily - no idea how
it is done with csh as I stopped using that decades ago).

If we decide to do that, then fixing that page to be rational would
be the right thing to do, if not, then deleting it.

I'll see if I can find out what the likely outcome of a discussion of
that will be (hopefully avoiding actually needing the discission again).
If there's a resolution, I will make it happen.

kre

ps: this one is not quite so important as the librumpuser issue...
And wrt that, I am not (I hope) a total cretin ... I would not have
objected if you had removed the (now) useless variable along with that
one line of code... (but what you did is fine).




Re: CVS commit: src/lib/librumpuser

2020-05-06 Thread Robert Elz
Date:Wed, 6 May 2020 07:30:17 +0200
From:Kamil Rytarowski 
Message-ID:  

  | I reverted my fix

It wasn't a fix, it simply made the problem go away, incorrectly.

If you want you can just delete the relevant lines (the ones you changed).
I've been running like that now since we started talking about this, and
everything appears fine.   You can put an  "OK kre" on the commit if you
do that.

I believe more changes are (or might be) needed - but that will correct
the buffer overflow problem, without breaking anything that will be visible
(and perhaps, nothing at all, I am still not certain).

  | from Match with a promise to see a better fix by kre@
  | but it never happened.

Yes, I got a bit side tracked with other issues, but this one is not
forgotten, just temporarily pushed down the stack a little.   It was
back near the top of the stack again, even before this small exchange
of messages.

kre



Re: CVS commit: src/sys

2020-04-18 Thread Robert Elz
Date:Sat, 18 Apr 2020 10:29:33 +
From:m...@netbsd.org
Message-ID:  <20200418102933.ga24...@homeworld.netbsd.org>

  | I feel like it's difficult to decide which is objectively better.

It all depends upon usage patterns, and objectives.

  | CVS encourages you to keep your local changes uncommitted, so they do
  | not show up in a change to RCSID at all.

Yes, that's a problem.   I'd actually prefer it if the compiler were to
add a note (of some kind, as long as it appears in the actual binary)
with the filename, mod date, and checksum, of every file it includes in
each compilation unit.   If there's also a VCS ID of some kind that can be
associated, even better.

The point was that a single identifier for the whole build simply
isn't detailed enough to be useful in many cases - it certainly wasn't
that CVS is better than something else.

  | But as a person with access to the repository, you are in a better
  | position in this case, because the DVCS will make it easy to go back to
  | the state of the tree given a hash, even after you add changes later.

I have had an off-list discussion with wiz about some of this - part of the
point is that often the person who wants to know whether a particular
version of a file was included has no reporitory access at all.   The
repository might not even still exist.   One of the questions we would
like to be able to answer is whether two different distributions were
built with the same version of some particular file - if that file is
later discovered to have been harbouring a bug.  And it would be nice to
be able to do that in isolation (no references to anything other than the
two (binary) distributions).

  | I imagine it isn't impossible to find 'closest parent which is also in
  | the remote' and embed it as well, mitigating the "outsider can't tell
  | how far you are" concern, if someone wants to pursue that.

To my mind, within reasonable cost bounds (space and effort) the more info
that is included the better.   It is always easy to simply ignore if not
needed, or redundant, but often impossible to reconstruct if missing.

kre



Re: CVS commit: src/sys

2020-04-17 Thread Robert Elz
Date:Fri, 17 Apr 2020 07:52:53 -0700
From:Jason Thorpe 
Message-ID:  <7e54033f-9f14-4db3-a11a-01d63cd92...@me.com>

  | The New Hotness (which isn't particularly new, at this point)
  | is to create branches and merge what you want into that branch.

What I don't understand, is how this single commit-id works in practice
(not how it is generated, I mean how it is used).   Say you've got some
local changes you're testing, maybe some ARM device driver (or related
stuff), and I have local changes as well (maybe some new sh code - I
actually have a lot of that, though most of it is no longer "new" and
quite possibly none of it will appear in public) - so we're both working
from different states of the overall tree.   Hence different ID's right?

Now imagine that while testing, some schedueller bug causes a panic,
or the ATF tests detect some (unrelated to both of us) failure that
shouldn't be happening (perhaps rump was neglected in someone's
changes from elsewhere, yet again).

If we both, along with someone running a pristine tree, all see this
failure, perhaps manifesting in slightly different ways, how do we
all determine that we're running the same versions of all of the
relevant files, and hence are probably all seeing the same bug?

Currently, with each file having its own version identfifier, it
is easy, but if everything is to share one single "it is this version"
ID, how can we know that we are all actually running the same version
of whatever code broke and is affecting all of us?

kre




Re: CVS commit: src/sys

2020-04-17 Thread Robert Elz
Date:Fri, 17 Apr 2020 15:37:33 +0200
From:Manuel Bouyer 
Message-ID:  <20200417133733.ga5...@antioche.eu.org>

  | And that would be a problem for me. I regulary update a single file to a
  | specific revision in a source tree.

Me too - I pull the current sh into NetBSD 8 (and I guess 9 now too,
though I haven't done that yet) and build it there for some people who
like to test and report bugs.

(Then I send them the binary to run on their system ... these tend to
be bare bones "base only" type systems whose only purpose in existing
is to test the shell.)

Everything is -8 except the contents of src/bin/sh which is -HEAD.
Not being able to easily do that kind of thing would be a nuisance.

I could obviously still do it, just by

cp -rp HEAD/src/bin/sh 8/src/bin

but methods like that are kind of ugly ... and certainly don't comply
with any assumption that the entire collection of files has a single
commit ID.

kre



Re: CVS commit: src/sys

2020-04-17 Thread Robert Elz
Date:Thu, 16 Apr 2020 18:04:04 -0700
From:Jason Thorpe 
Message-ID:  <432f538a-b863-441b-b4d0-0cd2e9d38...@me.com>

  | The sooner we get off of "things that use RCS semantics" the better.

For this, RCS and RCS semantics are irrelevant aren't they?   The only
relevance of RCS to any of this is the name of the macro (and given that
it is in almost every source file, I'd think it would need a very good
reason to warrant a change - it is just a name after all) and that the
contents of the string are currently (usually) automatically updated by RCS
(aka CVS).

The latter can easily change - the strings could contain any appropriate
identification data (commit ID's from whatever system is in use).

Using "RCS" in the name is also not really inappropriate, as while there
is a revision control system called just "Revision Control System" that's
also a generic name that applies to all of them (git, mercury, subversion,
cvs, even sccs, all included, along with RCS itself).

kre



Re: CVS commit: src/sys/netinet6

2020-04-12 Thread Robert Elz
Now that's a simpler fix than I imagined it would be...

kre



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

2020-04-05 Thread Robert Elz
Date:Sat, 4 Apr 2020 21:16:45 +
From:David Holland 
Message-ID:  <20200404211645.ga19...@netbsd.org>

  | 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?

You've never done something like

p = getenv("PATH"); /* NULL check omitted here */
while (q = strchr(p, ':')) {
*q = '\0';
/* use p to do a lookup, printf, or whatever */
*q = ':';
p = q + 1;
}
/* use p here too */

??

If you have, what's the difference, genenv() is also returning data
from libc, it could also be const char *, but isn't.

In neither case is this what would normally be called "internal data"
though, it isn't as if we're directly modifying the data structs malloc()
uses (that would be internal data) - these are the results returned from
libc to the application.

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

But we aren't returning a pointer to data that can't be modified,
libc doesn't care, one way or the other.   Apps shouldn't modify it
(shouldn't attempt to) because of portability concerns with other
implementations.If we did as Joerg suggested and mmapped
the message catalog pages for the relevant locale, and simply returned
a pointer to the relevant entry, one of those could be us.But isn't.

  | Anyway, this is moot because strerror is defined by C;

That was my point.   The entry in the BUGS section of the man page was
merely a rant, as this isn't (wasn't ever) something that can be fixed.

  | but because it *should* be const,
  | it should be (and is) declared with __aconst in string.h.
  |
  | Please don't change that.

Not planning anything like that - this was all about the man page.

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

I believe that what is in strerror(3) now is more or less aligned
with the intent (if not the actual language) of that.

Note that strsignal(3) doesn't contain a BUGS section ranting about
how it should really be const char *strsignal().

kre




Re: CVS commit: src/sys/modules/examples/fopsmapper

2020-04-01 Thread Robert Elz
Date:Wed, 1 Apr 2020 16:31:01 +0200
From:Kamil Rytarowski 
Message-ID:  

  | Does it look better?
  |
  | http://netbsd.org/~kamil/patch-00244-fopsmapper-PAGE_SIZE.txt

Apart from it needing to be (expressed in the relevant
make syntax, whatever that is)

if (WARNS > 3) WARNE=3

rather than simply WARNS=3 then as a hack type fix that looks fine.

But this is really a gcc bug, we shouldn't be needing to change our
sources at all because of this.

The signed vs unsigned comparison warning is because if the signed number
happens to be negative, when treated as unsigned it will compare larger
(usually) than the unsigned value - whereas most people expect negative
numbers to be less than positive ones (whatever the magnitudes).   Warning
for that potential problem is entirely reasonable.

But here we have a known positive (though signed) constant.   It can never
magically become negative, and so the problem can never exist.  There should
not (ever) be a warning for this case, it is simply stupid, and we should
not need to modify either code, or makefiles, to make it go away.

kre




Re: CVS commit: src/lib/librumpuser

2020-04-01 Thread Robert Elz
Date:Wed, 1 Apr 2020 15:54:15 +0200
From:Kamil Rytarowski 
Message-ID:  <969362d2-d93e-2cf4-7437-ab593ab11...@gmx.com>

  | Ping? This still breaks.

I am still working on it.Best I can tell at the minute is that the \0
is potentially needed (in a theoretical sense) but not by anything
operating rationally.

That is, when rump is used the strings will already always be \0 terminated
and the extra one added (the one that is off the end of the array) is never
needed, there's always an earlier one.

However, the relevant struct (that contains the string) comes from some
other process, and while if that process is running rump code, which is
what is intended to happen, all will be OK (I believe, I am not finished
checking all of that code), if it is something else, generating rump
packets, and passing them through, then we have no idea what will
be there, and the \0 termination cannot be guaranteed (and if we don't
do something, the rump process will eventually do bizarre things, that
out of the array \0 is currently preventing that possibility).

I see two reasonable paths forward here:

1. instead of adding the \0 off the end of the array, check that the
array is already \0 terminated (it should be, and always is in the ATF
test uses of rump - I ran the tests with a check in place, and it never
failed) - the \0 is always in the final byte of the array (the one you
overwrote in your earlier change, which meant that the changed line was
just a no-op, in practice, as suspected earlier.)

2. When we are reading an exec rump struct, allocate (and zero - the zero
part is already present) 1 byte more than will be received from the
sending process, so that the final byte will always remain as a \0, and
we will absolutely guarantee that the string will be \0 terminated (in
all normal cases it would end up terminated by two \0's).

If we do either of these, we don't need to waste time verifying that rump
always does send (in every case) a \0 terminated string (digging through the
code to work out where some of these structs get built is a slow process)
as the actual problem will be solved either way.

Solution 1 makes it an error, and the rup process will fail the exec if
the path isn't correctly \0 terminated.   Solution 2 does what the code
currently does (effectively) adding a \0 beyond the string that is received
from the sending process, but does it within the array bounds (by making the
array bigger) rather than outside them.

Opinions for which is better?

kre



Re: CVS commit: src/sys/modules/examples/fopsmapper

2020-04-01 Thread Robert Elz
Date:Wed, 1 Apr 2020 11:45:53 +
From:"Kamil Rytarowski" 
Message-ID:  <20200401114554.05167f...@cvs.netbsd.org>

  | Log Message:
  | Avoid comparison between signed and unsigned integer
  |
  | Cast PAGE_SIZE to size_t.

This kind of pedantry is going way too far, PAGE_SIZE is a compile
time constant (1 << PAGE_SHIFT) which is an int (and so signed,
nominally) but one which is known to be positive.

What is to be next?  Given an unsigned var (any unsigned type) 'u'
are we going to be required to write

if (u != 0U)
or
if (u != (unsigned)0)

instead of just
if (u != 0)
?

Get rid of the cast, it isn't needed in this case,
and anything that believes it is, is wrong (broken).

kre



Re: CVS commit: src/external/gpl3

2020-03-27 Thread Robert Elz
Date:Sat, 28 Mar 2020 11:46:29 +1100
From:matthew green 
Message-ID:  <15233.1585356...@splode.eterna.com.au>

  | can we just leave this as-is and let netbsd GCC people care?

Only if the GCC people do care, and understand the issue, and
implement what we want

Based entirely upon messages here, it seems as if they believe exactly
the opposite (or once did), eg: one of my systems has ...

-rw---  1 root  wheel  0 Aug  1  2019 /var/tmp/cc0CF9Uh.le
-rw---  1 root  wheel  0 Jun 15  2019 /var/tmp/cc0SLuqk.c
-rw---  1 root  wheel  94702 Aug  1  2019 /var/tmp/cc0uAn0K.ld
-rw---  1 root  wheel  0 Jun  1  2019 /var/tmp/cc2wYgEG.o
-rw---  1 root  wheel  0 May 21  2019 /var/tmp/cc2zHkbz.le
-rw---  1 root  wheel  0 Apr 29  2019 /var/tmp/cc4G3m88.le
-rw---  1 root  wheel  98595 May 21  2019 /var/tmp/cc4dzxVs.ld
-rw---  1 root  wheel  98595 Jun  3  2019 /var/tmp/cc5MqdQs.ld
-rw---  1 root  wheel  98595 Jun  1  2019 /var/tmp/cc5ROcTW.ld
-rw---  1 root  wheel  0 Aug  1  2019 /var/tmp/cc5sAGZG.c
-rw---  1 root  wheel  94702 Jul 10  2019 /var/tmp/cc73am4Y.ld
-rw---  1 root  wheel  0 Apr 22  2019 /var/tmp/ccAJcqpq.le
-rw---  1 root  wheel  0 Aug  1  2019 /var/tmp/ccCHeiPd.o
-rw---  1 root  wheel  0 Jul 10  2019 /var/tmp/ccDZoY7q.c
-rw---  1 root  wheel  0 Apr 29  2019 /var/tmp/ccEDt7pv.le
-rw---  1 root  wheel  0 Aug  1  2019 /var/tmp/ccGG3Duh.le
-rw---  1 root  wheel  0 Apr 18  2019 /var/tmp/ccIAAB5X.o
-rw---  1 root  wheel  0 Jul 10  2019 /var/tmp/ccIB2j2f.le
-rw---  1 root  wheel  0 Apr 18  2019 /var/tmp/ccJOlV73.c
-rw---  1 root  wheel  0 Jun  3  2019 /var/tmp/ccJgigog.c
-rw---  1 root  wheel  0 Jun 15  2019 /var/tmp/ccKW5VdP.le
-rw---  1 root  wheel  0 May  1  2019 /var/tmp/ccMI998l.o
-rw---  1 root  wheel  0 Apr 29  2019 /var/tmp/ccOLPHE9.o
-rw---  1 root  wheel  98595 Apr 29  2019 /var/tmp/ccP3twoE.ld
-rw---  1 root  wheel  0 Jun  3  2019 /var/tmp/ccSJQiCm.o
-rw---  1 root  wheel  0 Jun  3  2019 /var/tmp/ccUfI43y.le
-rw---  1 root  wheel  0 May 21  2019 /var/tmp/ccWJa4pg.c
-rw---  1 root  wheel  0 Apr 29  2019 /var/tmp/ccYvThNk.o
-rw---  1 root  wheel  0 Aug  1  2019 /var/tmp/cca0jZaH.c
-rw---  1 root  wheel  0 Apr 22  2019 /var/tmp/cccefbii.o
-rw---  1 root  wheel  0 May  1  2019 /var/tmp/cceQHHTx.le
-rw---  1 root  wheel  0 Jun  1  2019 /var/tmp/ccf4Nepq.c
-rw---  1 root  wheel  98595 Apr 29  2019 /var/tmp/ccg0gH6p.ld
-rw---  1 root  wheel  0 Apr 29  2019 /var/tmp/cch9cpUE.c
-rw---  1 root  wheel  0 May 21  2019 /var/tmp/ccia5LFm.o
-rw---  1 root  wheel  98595 May  1  2019 /var/tmp/cclxyp1r.ld
-rw---  1 root  wheel  98595 Apr 18  2019 /var/tmp/ccnS4e3R.ld
-rw---  1 root  wheel  0 Apr 29  2019 /var/tmp/cco6yYtf.c
-rw---  1 root  wheel  0 Aug  1  2019 /var/tmp/ccoXNE5d.o
-rw---  1 root  wheel  0 May  1  2019 /var/tmp/ccpQBE9f.c
-rw---  1 root  wheel  0 Jun 15  2019 /var/tmp/ccsxBUGu.o
-rw---  1 root  wheel  0 Apr 22  2019 /var/tmp/cctaeuee.c
-rw---  1 root  wheel  0 Apr 18  2019 /var/tmp/ccukTN0L.le
-rw---  1 root  wheel  98595 Apr 22  2019 /var/tmp/ccvEGOlm.ld
-rw---  1 root  wheel  0 Jun  1  2019 /var/tmp/ccwhr37c.le
-rw---  1 root  wheel  93664 Aug  1  2019 /var/tmp/ccxwbXEK.ld
-rw---  1 root  wheel  98595 Jun 15  2019 /var/tmp/ccyaWpXE.ld
-rw---  1 root  wheel  0 Jul 10  2019 /var/tmp/ccyxx75H.o

Those will be from system builds most likely interrupted by power fails
(no newer ones as I stopped doing builds there - that there are no
older ones suggests that the last time I went and manually cleaned up
was March or early April last year).

Trash like this that either needs manual cleaning, or purpose added
scripts, for no good reason, is absurd.   Even the (no longer true
anyway, but once was) "more space available in /var/tmp" is (and
always was) stupid for this purpose, even on the most ancient of
systems, /tmp was always made big enough for compiler temp files,
which aren't really ever all that huge - and while today we might
have lots of parallel compiles running, we also have bigger /tmp,
the ancient systems with 10Mb /tmp partitions couldn't really cope
with more than one or two compiles happening simultaneously (and
certainly wouldn't with the CPU/mem load imposed by a modern gcc).

So, if the gcc people are willing to (or maybe already have) switched
to /tmp for temporary files (in a way that can be overridden by setting
TMPDIR) then of course, we should just use their version.   If not, we
need to fix it.

And:  m...@eterna.com.au said (in a different message):
  | i don't like this "don't care about netbsd-8" feeling i'm hearing

I think you took Martin's words out of context.   I read that as
applying only to where gcc puts its temp files 

Re: CVS commit: src/external/gpl3

2020-03-26 Thread Robert Elz
Date:Thu, 26 Mar 2020 23:22:57 +
From:Andrew Doran 
Message-ID:  <20200326232257.gf27...@homeworld.netbsd.org>

  | > Modern CPUs like Ryzen Threadripper 3990X can execute that extra amount
  | > of instructions (around 600) in a single clock cycle.
  |
  | NetBSD-current would probably take longer to build on that AMD chip, than
  | Research Unix took to build on a PDP-11/70 in the late 1970s.  It still
  | counts.

Not only tat, but Kamil's measurements only count the kernel cost, not
the cost for the shell adding the var to the env on every exec, or the
additional cost on env var lookups of having to scan one extra var that
isn't the one being sought (50% of the time on average - the other lookups
find the var before the extra one) - and of course, all the lookups for
vars like LD_LIBRARY_PATH that have to scan everything, as those are almost
never present.

But discussion isn't worth the time it is taking.

kre




Re: CVS commit: src/external/gpl3

2020-03-26 Thread Robert Elz
Date:Thu, 26 Mar 2020 18:07:54 +0100
From:Kamil Rytarowski 
Message-ID:  <723e979c-cb16-1a39-a51e-9d756f372...@gmx.com>

  | But nonetheless, I'm trying to fix it un GCC without TMPDIR.

Not without, just without requiring.   Something like tempnam(3)
works - if TMPDIR is set, use it, if not, use /tmp.

kre



Re: CVS commit: src/external/gpl3

2020-03-26 Thread Robert Elz
Date:Thu, 26 Mar 2020 16:28:18 +0100
From:Kamil Rytarowski 
Message-ID:  <84460ebb-b4bf-f3ee-e51b-e27d0b6e2...@gmx.com>


  | That is negligible cost of getting TMPDIR propagated to most programs.

Sure, it isn't much, for one program, but when you're running tens of
thousands, it all adds up.

And, for most users, there should be no point - only those who need to
alter the default need to set it.

There has to be some fallback for programs run like

env -i prog

for TMPDIR (and everything else, like PATH, ...) if those defaults
are suitable for most users, then less leeds to be added to the environment.

kre



Re: CVS commit: src/external/gpl3

2020-03-26 Thread Robert Elz
Date:Thu, 26 Mar 2020 15:11:46 +0100
From:Kamil Rytarowski 
Message-ID:  <26eaf84f-616a-d48b-9a2f-7dd74cd69...@gmx.com>


  | Right. Addressing tools build independently (honoring TMPDIR?)

This we should do.

  | defining TMPDIR in /etc shell profile for common use inside the
  | distribution.

but this we should not.   The default (when TMPDIR is not specified)
should be what we would put there as the default - env vars to override
defaults should only be used by users who want something different, not
as the standard way of doing things.

EVery extra env var has a cost in every exec of absolutely everything.

kre



Re: CVS commit: src/external/gpl3

2020-03-25 Thread Robert Elz
Date:Wed, 25 Mar 2020 20:59:59 -0400
From:Greg Troxel 
Message-ID:  

  | I'll fourth this sentiment.

Me too (5th...)

The idea that /tmp is smaller than /var/tmp (or has less available space)
is based upon historic RAM sizes, my /tmp currently has about 10 times
as much available space as /var/tmp (when empty, the difference is more like
3:1 but /var/tmp (well, /var) has other stuff occupying space) - I run the
latter out from time to time, but never run out of /tmp space.

For current use, anything which is truly temporary shouid be in /tmp,
whatever the gcc upstream believe, their temporary files are just not
important enough to warrant keeping, and their ideas of available space
on modern systems are absurd.

kre



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

2020-03-25 Thread Robert Elz
Date:Wed, 25 Mar 2020 20:51:25 +
From:David Holland 
Message-ID:  <20200325205125.ga11...@netbsd.org>

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

Note that when strerror_l() was created, the opportunity to make that
return const char * existed - const existed in C by then, which it didn't
when strerror() was invented - but wasn't taken, as it was clear by its
very design that strerror_l had to return a pointer to writable memory,
so it was made char * and not const char *.

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

True, if the interface had been to return const char *.

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.

kre

ps: I've never seen, and cannot really imagine, an app that would ever want
to modify the result from strerror(), so none of this really matters anyway.



Re: CVS commit: src/lib/librumpuser

2020-03-25 Thread Robert Elz
Date:Tue, 24 Mar 2020 19:37:02 +0100
From:Kamil Rytarowski 
Message-ID:  <57a7e062-9af0-0be9-cb24-e155c5f83...@gmx.com>

  | ASan detects not a hypothetical, but factual momory corruption.

Yes, I know that - and I believed you from the start that there was a
buffer overrun there.

The issue is whether the offending line was useful for anything or not.

I am currently running ATF tests with that line simply deleted (actually,
replaced by a check that there is a \0 in the buffer somewhere already,
and abort() if not).

I'm expecting, for our ATF tests, that this is going to work fine (not abort),
which is why your "fix" looked correct - it certainly avoided the buffer
overrun, and was simply writing a \0 either on top of one that already was
present in the same byte (I am going to do another check to see if this was
the case next, but your asan output from this message suggests probably not)
or after an earlier \0 somewhere previously in the buffer (ie: in empty
unused space beyond the end of the string (which the asan output suggests
is the more likely case)).

Note that knowing that this is true of our tests, while useful info, proves
nothing - different data might stress the code in different ways, hence I
am going to find where (everywhere) the data is first constructed, and check
that it always guarantees \0 termination.   If there is, then the total
fix for the ASAN problem is to delete the offending line.   If not, the
correct fix is to make sure that the data is always correctly \0 terminated
-- and then delete the offending line.That's what I mean about "wait for
the real problem" - we need to find out whether the line that wrote beyond
the buffer end was there merely as a "I am going to treat this data like a
string, and bad things will happen if it isn't \0 terminated somehow, so
I will just stick a \0 after the buffer, just in case" type protection
mechanism, that was never really needed in the first place (a security
blanket), or whether there is some case where the code, given appropriate
data (say an exec that is MAXPATHLEN long, or something ... this is just
wild speculation of course) where it might currently be possible that no \0
is present, in which case that \0 added was saving things. and your fix was 
corrupting the data, and the correct fix is to make the buffer 1 byte bigger
so the \0 will fit (where the data is originally constructed).

We just don't know yet - and no amount of random testing will tell us (except
by fluke, and even that would just indicate that there is a real problem,
by managing to use data that triggers it, but probably not where in the code
is the base cause), it needs careful code reading.

This is why when the sanitisers find a problem, and it isn't obvious what
is the real cause (as opposed to the actual line that causes the problem)
you should avoid making random "fixes" to make the sanitiser report go away
(by no longer doing whatever it is complaining about - but not necessarily
implementing the original algorithm, whatever it was, correctly either).

This is an example of a case like that.   On the other hand, the other change
you committed at about the same time (the one with the iov where there was
an a && b test, where logically both have to be true for the code to be
executed, so it shouldn't matter which order, so programmers tend to put the
more likely to be false first, to save testing the less likely one when the
other is false - but in the case in questionb, when "b" was false, testing
"a" was accessing beyond the end of the memory.   Ie: the test should have
been b && a - and that's what you changted it to.   For that one, the cause
of the issue was obvious, and the fix correct - when you see ones that are
that simple, by all means, just fix them, as you did that time.

But if in doubt, file a PR - sanitiser detected bugs, while (at least often)
real bugs (ubsan not quite so much...) are rarely, if ever, critical fix
type - they have usually been present for years, harming no-one, so taking
a few extra days/weeks/months to arrive at the correct fix isn't really
doing much harm, usually.

kre



Re: CVS commit: src/lib/librumpuser

2020-03-24 Thread Robert Elz
Date:Tue, 24 Mar 2020 13:27:45 +0100
From:Kamil Rytarowski 
Message-ID:  <5ec1195a-f1c8-cd46-6a14-ea29da109...@gmx.com>

  | I patched it myself only when I reproduced the problems myself.

I have no doubt that there's a bug that needs fixing - it is the fix
proposed that is wrong.   My guess is that most probably it is simply
doing nothing useful (no harm, no good either) but I need to confirm
that.   If it is, the correct fix is simply to delete the line (both
times it was changed).   If not, there's a more serious problem elsewhere
that needs fixing elsewhere (after which the line can be deleted!)

  | OK. I will do it and please fix it in a better way.

Working on it now.

I haven't seen the revert yet, when I do I will commit the fix (or a fix,
this one is somewhat debatble what is correct, though what is there now
is obviously wrong ... but just like the one in question, while wrong, it
is, in practice, harmless, at least in any normal use of librump).

The fix for this issue needs to wait until the real problem the offending
line is there to deal with (if any, which I suspect is not the case) is
found.

kre



Re: CVS commit: src/lib/librumpuser

2020-03-23 Thread Robert Elz
Date:Tue, 24 Mar 2020 05:40:13 +0100
From:Kamil Rytarowski 
Message-ID:  


  | This patch was sitting in the tree since August 2019.

In your tree I assume you mean, it certainly hasn't been in mine.

Was a PR filed about the issue back then?   If so, shouldn't it have
been referenced in the commit message?If not, one should have been.

If that had (either initially, or in a later add on message) included
the suggested patch, I (and probably several others) would have told you
at the time it was not the correct thing to do - it took all of 30 seconds
of looking at the code to know that it is impossible to be the right fix.

  | This patch was tested to be operational.

I am not surprised at that at all - like I said (but I have still not
analysed things fully) I suspect that all it is doing is writing one \0
on top of a \0 that is already there, and that the correct patch would
have been to simply delete the offending line (and the comments attached
to it).

But that's still a guess.

  | I am aware that writing out of allocated buffer is for some people
  | considered as 'no bug'.

That's not what I said.   What I said was that while it is a bug, and
should be fixed, it was evidently harmless most of the time, as everything
has been working with this bug in it.

There are several possible problems that might exist here, perhaps (maybe
only in some extreme case) the buffer allocated is one byte too short, and
the correct fix would be to add a "+1" to some malloc() call elsewhere in
the code (I doubt that one, but it is possible).

Or perhaps the code that calculates commlen is incorrect, and it should
have had a "-1" included in the expression.

Or perhaps some code that is copying in the args is stopping just before
the \0 that terminates eact string, and failing to copy that one where it
should be, and this extra \0 appended is essential (in which case a correct
fix might be to copy the data into a slightly bigger buffer where there is
space for a \0 to be appended ... or change the arg copying code to include
the \0 rather than exclude it).   I doubt this one too, as I doubt that
things would have worked if this was the case - and your "it works with
the patch" makes it even less likely that this is the problem.

Or perhaps something else.

The point is that this needs to be analysed correctly, not just hacked at
to fix the problem, and that the line of code as it is now is either entirely
reduncant (my guess) or (perhaps only sometimes - and perhaps never in our
current tests) breaks things.

  | I'm fine to see it fixed in a better way than manually injected \0.

OK, then please revert that change (which cannot be the right fix, regardless
of what the right one really is) and file a PR, so it can be fixed properly.

kre

ps: while looking at this I spotted a (complely unrelated) bug in the same
file that should be fixed (no, sanitisers will never find this one, but
human reading easily might) - which I will fix, but I'd prefer to do that
after this fix is reverted, just so those two commit messages are adjacent.



Re: CVS commit: src/lib/librumpuser

2020-03-23 Thread Robert Elz
Date:Tue, 24 Mar 2020 01:56:56 +
From:"Kamil Rytarowski" 
Message-ID:  <20200324015656.33df1f...@cvs.netbsd.org>

  | Module Name:src
  | Committed By:   kamil
  | Date:   Tue Mar 24 01:56:56 UTC 2020
  |
  | Modified Files:
  | src/lib/librumpuser: rumpuser_sp.c
  |
  | Log Message:
  | Avoid buffer overflow
  |
  | Detected with ASan + RUMPKERNEL.

This "fix" cannot possibly be correct.

The code was (twice, the two cases are the same, so consider
just one of them)

/* ensure comm is 0-terminated */  
/* TODO: make sure it contains sensible chars? */
comm[commlen] = '\0';

where the "fix" is for the final line to be changed into

comm[commlen-1] = '\0';

Now I can see that the original may have been writing (perhaps just
sometimes, I haven't analysed it fully) one byte beyond the end of
the allocated space, and so something should be fixed, but your
change cannot possibly be the right way.

There are two possibilities here, in any particular case, either comm
was already nul-terminated, in which case the assignment isn't needed
at all, or it wasn't, in which case the changed code just destroyed the
final data byte by dumping a \0 on it - turning what was most probably
a harmless (though technically broken) one byte buffer overrun into
guaranteed broken code.

My guess is that the buffer is (always) already nul-terminated, and
the assignment was just paranoid over protection - and that the change
that was made simply overwrites one \0 with a different \0 (and so is
harmless, but stupid, the correct fix would be to delete the assignment
completely, similarly the "TODO" is nonsense, whatever the app decided
to exec() is its business, 'sensible' chars or not - either it works, or
doesn't, it isn't rump's business to second guess the client code).

I assume that because for exec() (which is what this is handling), the
arg strings are all \0 terminated - detecting that \0 is the only way the
arg copyin code knows where to stop.

If my guess is wrong, then the bug is where the args are collected. and
should be fixed there, not by plonking a \0 in here.

Kamil, once again, as I have asked before - if you don't have time to
fully analyse the problems that the sanitisers detect, please DO NOT
"fix" them - just file a PR and let someone who has the time deal with
it.Making the code generate no sanitiser bug reports by breaking it
is not our objective (I hope).   There was no great urgency to "fix"
this particular one, it doesn't seem to have been causing any real problems,
and could have easily waited a few days (or weeks, or even months) until
the correct solution was found.

kre



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

2020-03-17 Thread Robert Elz
Date:Tue, 17 Mar 2020 22:58:24 -0400
From:"Christos Zoulas" 
Message-ID:  <20200318025824.93b28f...@cvs.netbsd.org>

  | Log Message:
  | define un (pointed out by kre@)

The reason I didn't suggest that change, is that now un is unused
when USB_DEBUG is not defined.   At the very least it would need a
__debugused or whatever that #define for the relevant attribute is.

But for just a single use, it seemed simpler just to use the value
used to init the var that is (now) only used the once.

kre



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

2020-02-12 Thread Robert Elz
Date:Thu, 13 Feb 2020 02:45:44 +
From:Roy Marples 
Message-ID:  

  | My understanding was if it could be promoted to an int it would be.
  | So it size_t is bigger in bits than uint16_t and int is also bigger then 
  | promotion occurs and we then have signed vs unsigned.

You're right, but I think Joerg is even more right - or I certainly hope so.

The reasoning is that when the uint16_t gets promoted to int, since int
has more bits than the uint16_t the conversion is done by filling the
low 16 bits of the int with the uint16_t value, and zero filling the rest
(that is, value preserving).   You then have a signed int.  But it is
known to be >= 0.   When that is converted to a size_t (unsigned, and
here assumed to be at least as many bits as an int .. similar reasoning
applies if not) the signed int is sign extended to fit the size_t, and then
made unsigned.   "Sign exiended" here means zero fill, as we know the
value must be >= 0 (so the sign bit must be 0).

The only slightly weird case would be if int were 16 bits, but in that case,
a uint16_t would be an unsigned int, and no promotion to int ever happens,
it simply gets promoted directly to size_t.

If I had to guess, I'd say that gcc is losing the "must be >= 0" part of
the conversion from uint16_t to int, and assuming that the int might be
negative, which would be bad for converting to a size_t.

kre



Re: CVS commit: src/bin/sh

2020-02-06 Thread Robert Elz
Date:Fri, 7 Feb 2020 01:25:08 +
From:"Santhosh Raju" 
Message-ID:  <20200207012508.38c9cf...@cvs.netbsd.org>

  | bin/sh: Fixes -Werror=shadow causing build breaks.

Thanks for that.

kre



Re: CVS commit: src/sys/sys

2020-01-28 Thread Robert Elz
Date:Tue, 28 Jan 2020 16:40:27 +
From:"Andrew Doran" 
Message-ID:  <20200128164027.8558bf...@cvs.netbsd.org>

  | Log Message:
  | Put pri_t back to an int.  It looks like there might be a sign extension
  | issue somewhere but it's not worth the hassle trying to find it.

This changes the kernel internal ABI doesn't it?   It would have needed
a kernel version bump when the reverted change was made, and now needs
one it has been removed, doesn't it?

kre



Re: CVS commit: src/share/tmac

2019-12-25 Thread Robert Elz
Date:Wed, 25 Dec 2019 18:06:39 +0100
From:Steffen Nurpmeso 
Message-ID:  <20191225170639.ddhqn%stef...@sdaoden.eu>

  | Maybe the warning should really simply be removed, or only occur
  | if explicitly requested, i.e., remove the "el" warning bit from
  | the "all" level, just as is already done for some others; i guess
  | that would be the best.

Not really, a truly unmatched .el (the common cause is

.if ...
.el ...

) truly is an error, and a warning for that case is entirely
reasonable.   What's broken is generating it because of a chain
of .ie / .el .ie / .el .ie / .el

Each .ie needs a matching .el whether or not the .ie is actually
executed.

kre



Re: CVS commit: src/share/tmac

2019-12-23 Thread Robert Elz
Date:Mon, 23 Dec 2019 23:45:34 +0100
From:Steffen Nurpmeso 
Message-ID:  <20191223224534.8ufgy%stef...@sdaoden.eu>


  |  |Troff reads .ie and checks the condition.  The condition is true and
  |  |so the rest of the line is executed.  Then troff reads .el that
  |  |matches successful .ie, so the .el is discarded.
  |  |
  |  |Then it reads .el which is not matched with any .ie that troff has
  |  |seen.  It's usually silently discarded,

That is what is (always was) intended to happen.

  |  |but since we run with -ww we
  |  |get the warning about an unbalanced .el

That's a broken warning.   The code in a "discarded" .el needs to be
examined anyway (if it weren't, the \{ \} block to group the code
together wouldn't work, and when doing that, it can easily notice an
embedded .ie (it must be immediately after the .el so looking for it is
not hard) which will require another matching .el, and make sure that
things are correctly balanced.   That would be a useful wraning.

The
.ie
.el .ie
.el .ie
.el

idiom has been used in troff (including by troff's authors) forever.
Deciding to make that improper now, just because someone who doesn't
understand, or cannot work out how to handle it correctly, is absurd.

Perhaps (originally) the ".el .ie" combination could have been made
a macro of its own, it should really be treated as if it were (like
the "elif" word in sh), but adding more code to process another macro,
just to save (what would be) 3 input bytes per use wouldn't really
have been justifiable.

kre



Re: CVS commit: src/distrib/notes/common

2019-12-04 Thread Robert Elz
Sorry, really meant to send that just to wiz@, rather than the list...

kre



Re: CVS commit: src/distrib/notes/common

2019-12-04 Thread Robert Elz
Date:Wed, 4 Dec 2019 09:25:43 +
From:"Thomas Klausner" 
Message-ID:  <20191204092543.7c702f...@cvs.netbsd.org>

  | Module Name:src
  | Committed By:   wiz
  | Date:   Wed Dec  4 09:25:43 UTC 2019
  |
  | Modified Files:
  | src/distrib/notes/common: main
  |
  | Log Message:
  | Fix some typos.

This one almost certainly was not a typo, and the "fixed" version
is worse that what was there before.

Index: common/main
===
RCS file: /cvsroot/src/distrib/notes/common/main,v
retrieving revision 1.559
diff -u -r1.559 main
--- common/main 4 Dec 2019 09:25:43 -   1.559
+++ common/main 4 Dec 2019 12:55:34 -
@@ -909,7 +909,7 @@
 is your topic of interest; a list of possibly
 related man pages will be displayed.
 .
-.Ss Administrativa
+.Ss Administrivia
 .Pp
 .
 If you've got something to say, do so!


While "Administrivia" is not really a word, it is a common
slang expression indicating trivial issues that are related
to administration (I have no idea what Administrativa might be,
"administrative" is a word, but meaningless in that context).

kre



CVS commit: src/share/tmac

2019-11-26 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Nov 26 08:38:19 UTC 2019

Modified Files:
src/share/tmac: doc2html

Log Message:
PR toolchain/54715

Remove duplicate (incorrect) version of the .Lk macro, so the
earlier (fancier and functional) definition survives.

For now simply comment it out.  Sometime later this one should be
removed - but the two have been present since these macros were
first imported (1999) so leaving this visible (but removed) a
little longer shouldn't hurt.

I (believe) this one is the only duplicate of this form.

With this change the Lk macro in doc2html should work as it is designed
(but does not call other macros, and can only have punctuation following
the URL and (optional) anchor args (2nd arg is the anchor if it isn't
punctuation).

Tested by martin@


To generate a diff of this commit:
cvs rdiff -u -r1.67 -r1.68 src/share/tmac/doc2html

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/share/tmac

2019-11-26 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Nov 26 08:38:19 UTC 2019

Modified Files:
src/share/tmac: doc2html

Log Message:
PR toolchain/54715

Remove duplicate (incorrect) version of the .Lk macro, so the
earlier (fancier and functional) definition survives.

For now simply comment it out.  Sometime later this one should be
removed - but the two have been present since these macros were
first imported (1999) so leaving this visible (but removed) a
little longer shouldn't hurt.

I (believe) this one is the only duplicate of this form.

With this change the Lk macro in doc2html should work as it is designed
(but does not call other macros, and can only have punctuation following
the URL and (optional) anchor args (2nd arg is the anchor if it isn't
punctuation).

Tested by martin@


To generate a diff of this commit:
cvs rdiff -u -r1.67 -r1.68 src/share/tmac/doc2html

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/share/tmac/doc2html
diff -u src/share/tmac/doc2html:1.67 src/share/tmac/doc2html:1.68
--- src/share/tmac/doc2html:1.67	Tue Nov 22 00:36:49 2016
+++ src/share/tmac/doc2html	Tue Nov 26 08:38:19 2019
@@ -1,4 +1,4 @@
-.\" $NetBSD: doc2html,v 1.67 2016/11/22 00:36:49 kamil Exp $
+.\" $NetBSD: doc2html,v 1.68 2019/11/26 08:38:19 kre Exp $
 .\"
 .\" Copyright (c) 1998, 1999, 2008 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -788,12 +788,12 @@ AT UNIX\\$*
 .	as doc-pcresult \\*[an-eol]
 .	pc-fin
 ..
-.de Lk
-.	as doc-pcresult \\$1
-.	shift
-.	recurse \\$@
-.	pc-fin
-..
+.\".de Lk
+.\".	as doc-pcresult \\$1
+.\".	shift
+.\".	recurse \\$@
+.\".	pc-fin
+.\"..
 .de Mt
 .	as doc-pcresult \\$1
 .	shift



CVS commit: src/external/bsd/tmux/dist

2019-11-12 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Wed Nov 13 00:19:46 UTC 2019

Modified Files:
src/external/bsd/tmux/dist: tty-keys.c

Log Message:
Appease gcc.   Init "terminator".

It is plainly obvious that the init value cannot be used (the
var was never used uninit'd - could not be) but gcc apparently cannot
work that out.   Revert this if we ever get a compiler with a brain.


To generate a diff of this commit:
cvs rdiff -u -r1.11 -r1.12 src/external/bsd/tmux/dist/tty-keys.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/tmux/dist

2019-11-12 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Wed Nov 13 00:19:46 UTC 2019

Modified Files:
src/external/bsd/tmux/dist: tty-keys.c

Log Message:
Appease gcc.   Init "terminator".

It is plainly obvious that the init value cannot be used (the
var was never used uninit'd - could not be) but gcc apparently cannot
work that out.   Revert this if we ever get a compiler with a brain.


To generate a diff of this commit:
cvs rdiff -u -r1.11 -r1.12 src/external/bsd/tmux/dist/tty-keys.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/tmux/dist/tty-keys.c
diff -u src/external/bsd/tmux/dist/tty-keys.c:1.11 src/external/bsd/tmux/dist/tty-keys.c:1.12
--- src/external/bsd/tmux/dist/tty-keys.c:1.11	Tue Nov 12 21:02:28 2019
+++ src/external/bsd/tmux/dist/tty-keys.c	Wed Nov 13 00:19:46 2019
@@ -931,6 +931,7 @@ tty_keys_clipboard(__unused struct tty *
 		return (1);
 
 	/* Find the terminator if any. */
+	terminator = 0;	/* XXX: appease gcc (this value is never used) */
 	for (end = 5; end < len; end++) {
 		if (buf[end] == '\007') {
 			terminator = 1;



CVS commit: src/tests/usr.bin/printf

2019-11-12 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Nov 12 18:59:52 UTC 2019

Modified Files:
src/tests/usr.bin/printf: printf.sh

Log Message:
Add a missing ("quoting") '>' in an atf_fail error message string.
Since the tests don't (usually) fail no-one ever noticed the missing char.

That is, the "received this" and "expected this" strings were supposed
to appear in the output err message as "<>" but one of those
closing '>' chars was missing.

No-one should ever notice this change in normal operation, as the tests
are not intended to fail.


To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/tests/usr.bin/printf/printf.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/tests/usr.bin/printf

2019-11-12 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Nov 12 18:59:52 UTC 2019

Modified Files:
src/tests/usr.bin/printf: printf.sh

Log Message:
Add a missing ("quoting") '>' in an atf_fail error message string.
Since the tests don't (usually) fail no-one ever noticed the missing char.

That is, the "received this" and "expected this" strings were supposed
to appear in the output err message as "<>" but one of those
closing '>' chars was missing.

No-one should ever notice this change in normal operation, as the tests
are not intended to fail.


To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/tests/usr.bin/printf/printf.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/tests/usr.bin/printf/printf.sh
diff -u src/tests/usr.bin/printf/printf.sh:1.4 src/tests/usr.bin/printf/printf.sh:1.5
--- src/tests/usr.bin/printf/printf.sh:1.4	Sun Jul 21 15:25:59 2019
+++ src/tests/usr.bin/printf/printf.sh	Tue Nov 12 18:59:51 2019
@@ -1,4 +1,4 @@
-# $NetBSD: printf.sh,v 1.4 2019/07/21 15:25:59 kre Exp $
+# $NetBSD: printf.sh,v 1.5 2019/11/12 18:59:51 kre Exp $
 #
 # Copyright (c) 2018 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -118,7 +118,7 @@ expect()
 		(${WANT})
 		;;
 		(*) 
-		atf_fail "$* ... Expected <<${WANT}>> Received <<${RES}>"
+		atf_fail "$* ... Expected <<${WANT}>> Received <<${RES}>>"
 		;;
 		esac
 	fi



CVS commit: src/external/cddl/osnet/lib/libdtrace

2019-10-13 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sun Oct 13 10:07:27 UTC 2019

Modified Files:
src/external/cddl/osnet/lib/libdtrace: Makefile

Log Message:
This previously had -Wno-format-truncation so I am presuming it should
have been converted to GCC_NO_FORMAT_TRUNCATION rather than
GCC_NO_STRINGOP_TRUNCATION which is what happened.   This might unbreak
the build (olr at least get it further).


To generate a diff of this commit:
cvs rdiff -u -r1.24 -r1.25 src/external/cddl/osnet/lib/libdtrace/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/cddl/osnet/lib/libdtrace/Makefile
diff -u src/external/cddl/osnet/lib/libdtrace/Makefile:1.24 src/external/cddl/osnet/lib/libdtrace/Makefile:1.25
--- src/external/cddl/osnet/lib/libdtrace/Makefile:1.24	Sun Oct 13 07:28:08 2019
+++ src/external/cddl/osnet/lib/libdtrace/Makefile	Sun Oct 13 10:07:27 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: Makefile,v 1.24 2019/10/13 07:28:08 mrg Exp $
+#	$NetBSD: Makefile,v 1.25 2019/10/13 10:07:27 kre Exp $
 
 # $FreeBSD: head/cddl/lib/libdtrace/Makefile 314654 2017-03-04 11:30:04Z ngie $
 
@@ -107,7 +107,7 @@ CPPFLAGS+=	-I${OPENSOLARIS_SYS_DISTDIR}/
 CPPFLAGS+=	-I${OPENSOLARIS_SYS_DISTDIR}/uts/arm
 .PATH:		${.CURDIR}/../../dist/lib/libdtrace/arm
 .endif
-COPTS.dt_link.c		+= ${GCC_NO_STRINGOP_TRUNCATION}
+COPTS.dt_link.c		+= ${GCC_NO_FORMAT_TRUNCATION}
 
 LFLAGS+=-l
 



CVS commit: src/external/cddl/osnet/lib/libdtrace

2019-10-13 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sun Oct 13 10:07:27 UTC 2019

Modified Files:
src/external/cddl/osnet/lib/libdtrace: Makefile

Log Message:
This previously had -Wno-format-truncation so I am presuming it should
have been converted to GCC_NO_FORMAT_TRUNCATION rather than
GCC_NO_STRINGOP_TRUNCATION which is what happened.   This might unbreak
the build (olr at least get it further).


To generate a diff of this commit:
cvs rdiff -u -r1.24 -r1.25 src/external/cddl/osnet/lib/libdtrace/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/dev/pcmcia

2019-10-10 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Fri Oct 11 04:25:11 UTC 2019

Modified Files:
src/sys/dev/pcmcia: if_malo_pcmcia.c

Log Message:
Delete unused var (made redundant in previous commit).   Unbreak build.


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pcmcia/if_malo_pcmcia.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/pcmcia/if_malo_pcmcia.c
diff -u src/sys/dev/pcmcia/if_malo_pcmcia.c:1.22 src/sys/dev/pcmcia/if_malo_pcmcia.c:1.23
--- src/sys/dev/pcmcia/if_malo_pcmcia.c:1.22	Thu Oct 10 23:37:13 2019
+++ src/sys/dev/pcmcia/if_malo_pcmcia.c	Fri Oct 11 04:25:11 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_malo_pcmcia.c,v 1.22 2019/10/10 23:37:13 bad Exp $	*/
+/*	$NetBSD: if_malo_pcmcia.c,v 1.23 2019/10/11 04:25:11 kre Exp $	*/
 /*  $OpenBSD: if_malo.c,v 1.65 2009/03/29 21:53:53 sthen Exp $ */
 
 /*
@@ -18,7 +18,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: if_malo_pcmcia.c,v 1.22 2019/10/10 23:37:13 bad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_malo_pcmcia.c,v 1.23 2019/10/11 04:25:11 kre Exp $");
 
 #ifdef _MODULE
 #include 
@@ -1308,7 +1308,6 @@ cmalo_cmd_rsp_hwspec(struct malo_softc *
 	struct ieee80211com *ic = >sc_ic;
 	struct malo_cmd_header *hdr = (struct malo_cmd_header *)sc->sc_cmd;
 	struct malo_cmd_body_spec *body;
-	int i;
 
 	body = (struct malo_cmd_body_spec *)(hdr + 1);
 



CVS commit: src/sys/dev/pcmcia

2019-10-10 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Fri Oct 11 04:25:11 UTC 2019

Modified Files:
src/sys/dev/pcmcia: if_malo_pcmcia.c

Log Message:
Delete unused var (made redundant in previous commit).   Unbreak build.


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pcmcia/if_malo_pcmcia.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/lib/libc

2019-10-10 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 08:37:16 UTC 2019

Modified Files:
src/lib/libc/net: Makefile.inc
src/lib/libc/resolv: Makefile.inc
src/lib/libc/rpc: Makefile.inc

Log Message:
More cases to disable gcc-8 warnings only when we're using gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.89 -r1.90 src/lib/libc/net/Makefile.inc
cvs rdiff -u -r1.6 -r1.7 src/lib/libc/resolv/Makefile.inc
cvs rdiff -u -r1.24 -r1.25 src/lib/libc/rpc/Makefile.inc

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/lib/libc

2019-10-10 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 08:37:16 UTC 2019

Modified Files:
src/lib/libc/net: Makefile.inc
src/lib/libc/resolv: Makefile.inc
src/lib/libc/rpc: Makefile.inc

Log Message:
More cases to disable gcc-8 warnings only when we're using gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.89 -r1.90 src/lib/libc/net/Makefile.inc
cvs rdiff -u -r1.6 -r1.7 src/lib/libc/resolv/Makefile.inc
cvs rdiff -u -r1.24 -r1.25 src/lib/libc/rpc/Makefile.inc

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libc/net/Makefile.inc
diff -u src/lib/libc/net/Makefile.inc:1.89 src/lib/libc/net/Makefile.inc:1.90
--- src/lib/libc/net/Makefile.inc:1.89	Wed Oct  9 23:39:20 2019
+++ src/lib/libc/net/Makefile.inc	Thu Oct 10 08:37:16 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: Makefile.inc,v 1.89 2019/10/09 23:39:20 christos Exp $
+#	$NetBSD: Makefile.inc,v 1.90 2019/10/10 08:37:16 kre Exp $
 #	@(#)Makefile.inc	8.2 (Berkeley) 9/5/93
 
 # net sources
@@ -30,7 +30,7 @@ LPREFIX=_nsyy
 YPREFIX=_nsyy
 YHEADER=1
 
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.getaddrinfo.c += -Wno-error=stringop-overflow
 .endif
 

Index: src/lib/libc/resolv/Makefile.inc
diff -u src/lib/libc/resolv/Makefile.inc:1.6 src/lib/libc/resolv/Makefile.inc:1.7
--- src/lib/libc/resolv/Makefile.inc:1.6	Wed Oct  9 23:39:20 2019
+++ src/lib/libc/resolv/Makefile.inc	Thu Oct 10 08:37:16 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: Makefile.inc,v 1.6 2019/10/09 23:39:20 christos Exp $
+#	$NetBSD: Makefile.inc,v 1.7 2019/10/10 08:37:16 kre Exp $
 
 # net sources
 .PATH: ${.CURDIR}/resolv
@@ -10,6 +10,6 @@ SRCS+=	h_errno.c herror.c res_comp.c res
 # For COMPAT__RES
 SRCS+=	res_compat.c
 
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.res_query.c += -Wno-error=stringop-overflow
 .endif

Index: src/lib/libc/rpc/Makefile.inc
diff -u src/lib/libc/rpc/Makefile.inc:1.24 src/lib/libc/rpc/Makefile.inc:1.25
--- src/lib/libc/rpc/Makefile.inc:1.24	Wed Oct  9 23:39:20 2019
+++ src/lib/libc/rpc/Makefile.inc	Thu Oct 10 08:37:16 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: Makefile.inc,v 1.24 2019/10/09 23:39:20 christos Exp $
+#	$NetBSD: Makefile.inc,v 1.25 2019/10/10 08:37:16 kre Exp $
 
 # librpc sources
 .PATH:	${.CURDIR}/rpc
@@ -18,7 +18,7 @@ SRCS+=	auth_none.c auth_unix.c authunix_
 
 CPPFLAGS+=	-DPORTMAP
 
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.clnt_bcast.c += -Wno-error=cast-function-type
 COPTS.clnt_generic.c += -Wno-error=cast-function-type
 COPTS.clnt_vc.c += -Wno-error=cast-function-type



CVS commit: src/lib/libpam/modules/pam_lastlog

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:39:07 UTC 2019

Modified Files:
src/lib/libpam/modules/pam_lastlog: Makefile

Log Message:
Only exclude gcc-8 warnings if the gcc we're using is gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 src/lib/libpam/modules/pam_lastlog/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/lib/libpam/modules/pam_lastlog

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:39:07 UTC 2019

Modified Files:
src/lib/libpam/modules/pam_lastlog: Makefile

Log Message:
Only exclude gcc-8 warnings if the gcc we're using is gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 src/lib/libpam/modules/pam_lastlog/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libpam/modules/pam_lastlog/Makefile
diff -u src/lib/libpam/modules/pam_lastlog/Makefile:1.9 src/lib/libpam/modules/pam_lastlog/Makefile:1.10
--- src/lib/libpam/modules/pam_lastlog/Makefile:1.9	Wed Oct  9 22:05:35 2019
+++ src/lib/libpam/modules/pam_lastlog/Makefile	Thu Oct 10 02:39:07 2019
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.9 2019/10/09 22:05:35 christos Exp $
+# $NetBSD: Makefile,v 1.10 2019/10/10 02:39:07 kre Exp $
 # Copyright 2001 Mark R V Murray
 # All rights reserved.
 #
@@ -35,6 +35,6 @@ LIBDPLIBS+=	util	${.CURDIR}/../../../lib
 
 .include "${.CURDIR}/../mod.mk"
 
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.pam_lastlog.c += -Wno-stringop-truncation
 .endif



CVS commit: src/lib/libpam/libpam

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:37:40 UTC 2019

Modified Files:
src/lib/libpam/libpam: Makefile

Log Message:
Only exclude gcc-8 warnings when the gcc we're using is gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.20 -r1.21 src/lib/libpam/libpam/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/lib/libpam/libpam

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:37:40 UTC 2019

Modified Files:
src/lib/libpam/libpam: Makefile

Log Message:
Only exclude gcc-8 warnings when the gcc we're using is gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.20 -r1.21 src/lib/libpam/libpam/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libpam/libpam/Makefile
diff -u src/lib/libpam/libpam/Makefile:1.20 src/lib/libpam/libpam/Makefile:1.21
--- src/lib/libpam/libpam/Makefile:1.20	Wed Oct  9 22:05:09 2019
+++ src/lib/libpam/libpam/Makefile	Thu Oct 10 02:37:40 2019
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.20 2019/10/09 22:05:09 christos Exp $
+# $NetBSD: Makefile,v 1.21 2019/10/10 02:37:40 kre Exp $
 #-
 # Copyright (c) 1998 Juniper Networks, Inc.
 # All rights reserved.
@@ -204,7 +204,7 @@ openpam_static_modules.o: openpam_static
 	openpam_static.o ${STATIC_MODULE_LIBS}
 
 CWARNFLAGS.clang+=	-Wno-error=tautological-pointer-compare
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.openpam_dynamic.c += -Wno-error=cast-function-type
 .endif
 



CVS commit: src/external/cddl/osnet/lib/libdtrace

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:35:45 UTC 2019

Modified Files:
src/external/cddl/osnet/lib/libdtrace: Makefile

Log Message:
Only exclude gcc-8 warnings when the gcc we're using is gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/external/cddl/osnet/lib/libdtrace/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/cddl/osnet/lib/libdtrace/Makefile
diff -u src/external/cddl/osnet/lib/libdtrace/Makefile:1.22 src/external/cddl/osnet/lib/libdtrace/Makefile:1.23
--- src/external/cddl/osnet/lib/libdtrace/Makefile:1.22	Wed Oct  9 21:49:50 2019
+++ src/external/cddl/osnet/lib/libdtrace/Makefile	Thu Oct 10 02:35:45 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: Makefile,v 1.22 2019/10/09 21:49:50 christos Exp $
+#	$NetBSD: Makefile,v 1.23 2019/10/10 02:35:45 kre Exp $
 
 # $FreeBSD: head/cddl/lib/libdtrace/Makefile 314654 2017-03-04 11:30:04Z ngie $
 
@@ -85,7 +85,7 @@ COPTS.dt_printf.c	+= -Wno-stack-protecto
 COPTS.dt_program.c	+= -Wno-stack-protector
 COPTS.dt_provider.c	+= -Wno-stack-protector
 COPTS.dt_subr.c		+= -Wno-stack-protector
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.dt_lex.c		+= -Wno-error=stringop-truncation
 COPTS.dt_pid.c		+= -Wno-error=stringop-truncation
 .endif



CVS commit: src/external/cddl/osnet/lib/libdtrace

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:35:45 UTC 2019

Modified Files:
src/external/cddl/osnet/lib/libdtrace: Makefile

Log Message:
Only exclude gcc-8 warnings when the gcc we're using is gcc>=8


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/external/cddl/osnet/lib/libdtrace/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/bsd/fetch/lib

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:30:43 UTC 2019

Modified Files:
src/external/bsd/fetch/lib: Makefile

Log Message:
Only exclude ggc-8 warnings when the gcc we're using is gcc 8 or more.


To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 src/external/bsd/fetch/lib/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/bsd/fetch/lib/Makefile
diff -u src/external/bsd/fetch/lib/Makefile:1.13 src/external/bsd/fetch/lib/Makefile:1.14
--- src/external/bsd/fetch/lib/Makefile:1.13	Wed Oct  9 21:19:28 2019
+++ src/external/bsd/fetch/lib/Makefile	Thu Oct 10 02:30:43 2019
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.13 2019/10/09 21:19:28 christos Exp $
+# $NetBSD: Makefile,v 1.14 2019/10/10 02:30:43 kre Exp $
 
 LIB=		fetch
 SRCS=		fetch.c common.c ftp.c http.c file.c
@@ -36,7 +36,7 @@ httperr.h: ${LIBFETCHDIR}/http.errors ${
 CFLAGS+=	-Wno-macro-redefined # _REENTRANT redefined
 .endif
 
-.if ${ACTIVE_CC} == "gcc"
+.if defined(HAVE_GCC) && ${HAVE_GCC} >= 8 && ${ACTIVE_CC} == "gcc"
 COPTS.http.c += -Wno-error=stringop-truncation
 .endif
 



CVS commit: src/external/bsd/fetch/lib

2019-10-09 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Thu Oct 10 02:30:43 UTC 2019

Modified Files:
src/external/bsd/fetch/lib: Makefile

Log Message:
Only exclude ggc-8 warnings when the gcc we're using is gcc 8 or more.


To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 src/external/bsd/fetch/lib/Makefile

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:53:57 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Remove a (completely harmless) duplicate assignment introduced in a
code merge from FreeBSD in 2017.   NFC.

Pointed out by Roland Illig.


To generate a diff of this commit:
cvs rdiff -u -r1.133 -r1.134 src/bin/sh/expand.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/sh/expand.c
diff -u src/bin/sh/expand.c:1.133 src/bin/sh/expand.c:1.134
--- src/bin/sh/expand.c:1.133	Tue Oct  8 03:52:44 2019
+++ src/bin/sh/expand.c	Tue Oct  8 03:53:57 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $	*/
+/*	$NetBSD: expand.c,v 1.134 2019/10/08 03:53:57 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c	8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.134 2019/10/08 03:53:57 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -1955,7 +1955,6 @@ patmatch(const char *pattern, const char
 			}
 			/* end shortcut */
 
-			invert = 0;
 			savep = p, saveq = q;
 			invert = 0;
 			if (*p == '!' || *p == '^') {



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:53:57 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Remove a (completely harmless) duplicate assignment introduced in a
code merge from FreeBSD in 2017.   NFC.

Pointed out by Roland Illig.


To generate a diff of this commit:
cvs rdiff -u -r1.133 -r1.134 src/bin/sh/expand.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:52:44 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Open code the validity test & copy of the character class name in
a bracket expression in a pattern (ie: [[:THISNAME:]]).   Previously
the code used strspn() to look for invalid chars in the name, and
then memcpy(), now we do the test and copy a character at a time.
This might, or might not, be faster, but it now correctly handles
\ quoted characters in the name (' and " quoting were already
dealt with, \ was too in an earlier version, but when the \ handling
changes were made, this piece of code broke).

Not exactly a vital bug fix (who writes [[:\alpha:]] or similar?)
but it should work correctly regardless of how obscure the usage is.

Problem noted by Harald van Dijk

XXX pullup -9


To generate a diff of this commit:
cvs rdiff -u -r1.132 -r1.133 src/bin/sh/expand.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/sh/expand.c
diff -u src/bin/sh/expand.c:1.132 src/bin/sh/expand.c:1.133
--- src/bin/sh/expand.c:1.132	Wed Apr 10 08:13:11 2019
+++ src/bin/sh/expand.c	Tue Oct  8 03:52:44 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: expand.c,v 1.132 2019/04/10 08:13:11 kre Exp $	*/
+/*	$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c	8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.132 2019/04/10 08:13:11 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -1792,24 +1792,44 @@ match_charclass(const char *p, wchar_t c
 	char name[20];
 	char *nameend;
 	wctype_t cclass;
+	char *q;
 
 	*end = NULL;
 	p++;
+	q = [0];
 	nameend = strstr(p, ":]");
 	if (nameend == NULL || nameend == p)	/* not a valid class */
 		return 0;
 
-	if (!is_alpha(*p) || strspn(p,		/* '_' is a local extension */
-	"0123456789"  "_"
-	"abcdefghijklmnopqrstuvwxyz"
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZ") != (size_t)(nameend - p))
+	if (*p == CTLESC) {
+		if (*++p == CTLESC)
+			return 0;
+		if (p == nameend)
+			return 0;
+	}
+	if (!is_alpha(*p))
 		return 0;
+	while (p < nameend) {
+		if (*p == CTLESC) {
+			p++;
+			if (p == nameend)
+return 0;
+		}
+		if (!is_in_name(*p))	/* '_' is a local extension */
+			return 0;
+		if (q < [sizeof name])
+			*q++ = *p++;
+		else
+			p++;
+	}
 
 	*end = nameend + 2;		/* committed to it being a char class */
-	if ((size_t)(nameend - p) >= sizeof(name))	/* but too long */
-		return 0;/* so no match */
-	memcpy(name, p, nameend - p);
-	name[nameend - p] = '\0';
+
+	if (q < [sizeof name])	/* a usable name found */
+		*q++ = '\0';
+	else/* too long, valid, but no match */
+		return 0;
+
 	cclass = wctype(name);
 	/* An unknown class matches nothing but is valid nevertheless. */
 	if (cclass == 0)



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:52:44 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Open code the validity test & copy of the character class name in
a bracket expression in a pattern (ie: [[:THISNAME:]]).   Previously
the code used strspn() to look for invalid chars in the name, and
then memcpy(), now we do the test and copy a character at a time.
This might, or might not, be faster, but it now correctly handles
\ quoted characters in the name (' and " quoting were already
dealt with, \ was too in an earlier version, but when the \ handling
changes were made, this piece of code broke).

Not exactly a vital bug fix (who writes [[:\alpha:]] or similar?)
but it should work correctly regardless of how obscure the usage is.

Problem noted by Harald van Dijk

XXX pullup -9


To generate a diff of this commit:
cvs rdiff -u -r1.132 -r1.133 src/bin/sh/expand.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/doc

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 02:47:07 UTC 2019

Modified Files:
src/doc: 3RDPARTY CHANGES

Log Message:
Note tzdata2019c


To generate a diff of this commit:
cvs rdiff -u -r1.1654 -r1.1655 src/doc/3RDPARTY
cvs rdiff -u -r1.2589 -r1.2590 src/doc/CHANGES

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/doc/3RDPARTY
diff -u src/doc/3RDPARTY:1.1654 src/doc/3RDPARTY:1.1655
--- src/doc/3RDPARTY:1.1654	Mon Oct  7 00:27:21 2019
+++ src/doc/3RDPARTY	Tue Oct  8 02:47:07 2019
@@ -1,4 +1,4 @@
-#	$NetBSD: 3RDPARTY,v 1.1654 2019/10/07 00:27:21 christos Exp $
+#	$NetBSD: 3RDPARTY,v 1.1655 2019/10/08 02:47:07 kre Exp $
 #
 # This file contains a list of the software that has been integrated into
 # NetBSD where we are not the primary maintainer.
@@ -1398,8 +1398,8 @@ Notes:
 Added changes from a5 -> a12 manually.
 
 Package:	tz
-Version:	tzcode2019b / tzdata2019a
-Current Vers:	tzcode2019b / tzdata2019b
+Version:	tzcode2019b / tzdata2019c
+Current Vers:	tzcode2019c / tzdata2019c
 Maintainer:	Paul Eggert 
 Archive Site:	ftp://ftp.iana.org/tz/releases/
 Archive Site:	ftp://munnari.oz.au/pub/oldtz/

Index: src/doc/CHANGES
diff -u src/doc/CHANGES:1.2589 src/doc/CHANGES:1.2590
--- src/doc/CHANGES:1.2589	Mon Oct  7 00:27:21 2019
+++ src/doc/CHANGES	Tue Oct  8 02:47:07 2019
@@ -1,4 +1,4 @@
-# LIST OF CHANGES FROM LAST RELEASE:			<$Revision: 1.2589 $>
+# LIST OF CHANGES FROM LAST RELEASE:			<$Revision: 1.2590 $>
 #
 #
 # [Note: This file does not mention every change made to the NetBSD source tree.
@@ -51,3 +51,4 @@ Changes from NetBSD 9.0 to NetBSD 10.0:
 	tcpdump(8): Import 4.9.3. [christos 20191001]
 	rescue(8): Moved into a separate, rescue-only set. [maya 20191001]
 	byacc: Update to 20190617. [christos 20191006]
+	tzdata updates from 2019a to 2019c (incl 2019b) [kre 20191008]



CVS commit: src/doc

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 02:47:07 UTC 2019

Modified Files:
src/doc: 3RDPARTY CHANGES

Log Message:
Note tzdata2019c


To generate a diff of this commit:
cvs rdiff -u -r1.1654 -r1.1655 src/doc/3RDPARTY
cvs rdiff -u -r1.2589 -r1.2590 src/doc/CHANGES

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/public-domain/tz/dist

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 02:44:35 UTC 2019

Modified Files:
src/external/public-domain/tz/dist: TZDATA_VERSION

Log Message:
Merge tzdata2019c


To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/external/public-domain/tz/dist/TZDATA_VERSION

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/external/public-domain/tz/dist

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 02:44:35 UTC 2019

Modified Files:
src/external/public-domain/tz/dist: TZDATA_VERSION

Log Message:
Merge tzdata2019c


To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/external/public-domain/tz/dist/TZDATA_VERSION

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/public-domain/tz/dist/TZDATA_VERSION
diff -u src/external/public-domain/tz/dist/TZDATA_VERSION:1.17 src/external/public-domain/tz/dist/TZDATA_VERSION:1.18
--- src/external/public-domain/tz/dist/TZDATA_VERSION:1.17	Tue Mar 26 10:06:49 2019
+++ src/external/public-domain/tz/dist/TZDATA_VERSION	Tue Oct  8 02:44:35 2019
@@ -1 +1 @@
-tzdata-2019a
+tzdata-2019c



CVS import: src/external/public-domain/tz/dist

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 02:44:05 UTC 2019

Update of /cvsroot/src/external/public-domain/tz/dist
In directory ivanova.netbsd.org:/tmp/cvs-serv29639

Log Message:
Import tzdata2019c from ftp://ftp.iana.org/tz/releases/tzdata2019c.tar.gz

Summary of changes in tzdata2019c (2019-09-11 08:59:48 -0700):
Fiji observes DST from 2019-11-10 to 2020-01-12
Norfolk Island starts observing Australian-style DST

Plus historic corrections to time in Turkey (1940-85)
South Korea (1948-51) Detroit (US) (1967-8), Perry County
(Indiana, US) (pre 1970) Edmonton (CA) (1967, 1969)
Vancouver (CA) (1946), Vienna (AT) (1946), Kaliningrad (1945-6).
Louisville (US) (1946-50).  Brussles (BE) (1892).
Hong Kong Winter Time (1941) now listed as being "DST".

Summary of changes in tzdata2019b (2019-07-01 00:09:53 -0700):

Brazil no longer observes DST
Predictions for Morocco extended to 2087.
Panestine (March 2019) time zone change date corrected
(and guesses for future transitions revised).

Historic updates:  Honk Kong (1941 - 1947), Italy (1866).

Status:

Vendor Tag: TZDATA
Release Tags:   TZDATA2019C

U src/external/public-domain/tz/dist/leap-seconds.list
U src/external/public-domain/tz/dist/calendars
U src/external/public-domain/tz/dist/CONTRIBUTING
U src/external/public-domain/tz/dist/LICENSE
U src/external/public-domain/tz/dist/Makefile
U src/external/public-domain/tz/dist/NEWS
U src/external/public-domain/tz/dist/README
U src/external/public-domain/tz/dist/theory.html
U src/external/public-domain/tz/dist/version
U src/external/public-domain/tz/dist/africa
U src/external/public-domain/tz/dist/antarctica
U src/external/public-domain/tz/dist/asia
U src/external/public-domain/tz/dist/australasia
U src/external/public-domain/tz/dist/europe
U src/external/public-domain/tz/dist/northamerica
U src/external/public-domain/tz/dist/southamerica
U src/external/public-domain/tz/dist/etcetera
U src/external/public-domain/tz/dist/systemv
U src/external/public-domain/tz/dist/factory
U src/external/public-domain/tz/dist/backward
U src/external/public-domain/tz/dist/pacificnew
U src/external/public-domain/tz/dist/backzone
U src/external/public-domain/tz/dist/iso3166.tab
U src/external/public-domain/tz/dist/checklinks.awk
U src/external/public-domain/tz/dist/zone.tab
U src/external/public-domain/tz/dist/leapseconds
U src/external/public-domain/tz/dist/yearistype.sh
U src/external/public-domain/tz/dist/zone1970.tab
U src/external/public-domain/tz/dist/leapseconds.awk
U src/external/public-domain/tz/dist/checktab.awk
U src/external/public-domain/tz/dist/zoneinfo2tdf.pl
U src/external/public-domain/tz/dist/ziguard.awk
U src/external/public-domain/tz/dist/zishrink.awk

No conflicts created by this import



CVS import: src/external/public-domain/tz/dist

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 02:44:05 UTC 2019

Update of /cvsroot/src/external/public-domain/tz/dist
In directory ivanova.netbsd.org:/tmp/cvs-serv29639

Log Message:
Import tzdata2019c from ftp://ftp.iana.org/tz/releases/tzdata2019c.tar.gz

Summary of changes in tzdata2019c (2019-09-11 08:59:48 -0700):
Fiji observes DST from 2019-11-10 to 2020-01-12
Norfolk Island starts observing Australian-style DST

Plus historic corrections to time in Turkey (1940-85)
South Korea (1948-51) Detroit (US) (1967-8), Perry County
(Indiana, US) (pre 1970) Edmonton (CA) (1967, 1969)
Vancouver (CA) (1946), Vienna (AT) (1946), Kaliningrad (1945-6).
Louisville (US) (1946-50).  Brussles (BE) (1892).
Hong Kong Winter Time (1941) now listed as being "DST".

Summary of changes in tzdata2019b (2019-07-01 00:09:53 -0700):

Brazil no longer observes DST
Predictions for Morocco extended to 2087.
Panestine (March 2019) time zone change date corrected
(and guesses for future transitions revised).

Historic updates:  Honk Kong (1941 - 1947), Italy (1866).

Status:

Vendor Tag: TZDATA
Release Tags:   TZDATA2019C

U src/external/public-domain/tz/dist/leap-seconds.list
U src/external/public-domain/tz/dist/calendars
U src/external/public-domain/tz/dist/CONTRIBUTING
U src/external/public-domain/tz/dist/LICENSE
U src/external/public-domain/tz/dist/Makefile
U src/external/public-domain/tz/dist/NEWS
U src/external/public-domain/tz/dist/README
U src/external/public-domain/tz/dist/theory.html
U src/external/public-domain/tz/dist/version
U src/external/public-domain/tz/dist/africa
U src/external/public-domain/tz/dist/antarctica
U src/external/public-domain/tz/dist/asia
U src/external/public-domain/tz/dist/australasia
U src/external/public-domain/tz/dist/europe
U src/external/public-domain/tz/dist/northamerica
U src/external/public-domain/tz/dist/southamerica
U src/external/public-domain/tz/dist/etcetera
U src/external/public-domain/tz/dist/systemv
U src/external/public-domain/tz/dist/factory
U src/external/public-domain/tz/dist/backward
U src/external/public-domain/tz/dist/pacificnew
U src/external/public-domain/tz/dist/backzone
U src/external/public-domain/tz/dist/iso3166.tab
U src/external/public-domain/tz/dist/checklinks.awk
U src/external/public-domain/tz/dist/zone.tab
U src/external/public-domain/tz/dist/leapseconds
U src/external/public-domain/tz/dist/yearistype.sh
U src/external/public-domain/tz/dist/zone1970.tab
U src/external/public-domain/tz/dist/leapseconds.awk
U src/external/public-domain/tz/dist/checktab.awk
U src/external/public-domain/tz/dist/zoneinfo2tdf.pl
U src/external/public-domain/tz/dist/ziguard.awk
U src/external/public-domain/tz/dist/zishrink.awk

No conflicts created by this import



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

2019-09-22 Thread Robert Elz
Date:Sun, 22 Sep 2019 07:23:00 +0100
From:Nick Hudson 
Message-ID:  <793d2380-8d1a-78ab-3682-0468aea0d...@gmx.co.uk>

  | I was merely pointing out that it exists already.

Understood, and thanks - at a minimum that will avoid adding
it with a different name.

  | I'm happy to help adding it to other ports.

There's certainly no harm in that, for future use, but for right
now there's no real need.

kre



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

2019-09-21 Thread Robert Elz
Date:Sun, 22 Sep 2019 01:23:41 +0700
From:Robert Elz 
Message-ID:  <8235.1569090...@jinx.noi.kre.to>

  | we'd need it in all the other ports before it can be used in MD code.

I meant MI code of course...

kre



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

2019-09-21 Thread Robert Elz
Date:Sat, 21 Sep 2019 19:06:15 +0100
From:Nick Hudson 
Message-ID:  

  | 
http://src.illumos.org/source/search?q=PRIxBUSADDR=PRIxBUSADDRnetbsd-src

That shows that mips and arm have PRIxBUSADDR - we'd need it in all the
other ports before it can be used in MD code.

kre



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

2019-09-21 Thread Robert Elz
Date:Sat, 21 Sep 2019 09:26:21 -0700
From:Jason Thorpe 
Message-ID:  

  | Should we make a PRIxxx macro for it?

[since I deleted the context: "it" is bus_addr_t]

Perhaps, for this particular case it doesn't really matter, but if
code is likely to be printing buss_addr_t type values often enough
that it might make a difference, that might be worthwhile.

kre



CVS commit: src/tests/fs/vfs

2019-09-21 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sat Sep 21 14:25:42 UTC 2019

Modified Files:
src/tests/fs/vfs: t_ro.c

Log Message:
Initialise the sometvs array of struct timeval that is to be used to
validate that utimes() cannot update the times of a file on a read only
filesystem.   The values are never actually used, but since
src/sys/kern/vfs_syscalls.c 1.535
they are validated for sanity, and the syscall returns EINVAL if the
values passed are invalid (tv_usec <0 or >= 100).  If that happens
we don't get as far as the test which produces the EROFS that is expected
from this test (these tests - one for each filesystem type).

So, init the timeval structs (just to 0, the values will still not be
used) so that the EINVAL doesn't bite us before we're eaten by the EROFS
which is the way we're supposed to die.

If the syscall API args were labelled as "const" the compiler probably
would have caught the use of uninit'd vars and complained much sooner.


To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/tests/fs/vfs/t_ro.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/tests/fs/vfs/t_ro.c
diff -u src/tests/fs/vfs/t_ro.c:1.7 src/tests/fs/vfs/t_ro.c:1.8
--- src/tests/fs/vfs/t_ro.c:1.7	Tue Jul 16 17:29:17 2019
+++ src/tests/fs/vfs/t_ro.c	Sat Sep 21 14:25:42 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: t_ro.c,v 1.7 2019/07/16 17:29:17 martin Exp $	*/
+/*	$NetBSD: t_ro.c,v 1.8 2019/09/21 14:25:42 kre Exp $	*/
 
 /*-
  * Copyright (c) 2010 The NetBSD Foundation, Inc.
@@ -120,7 +120,7 @@ fileio(const atf_tc_t *tc, const char *m
 static void
 attrs(const atf_tc_t *tc, const char *mp)
 {
-	struct timeval sometvs[2];
+	struct timeval sometvs[2] = { {0,0}, {0,0} };
 	struct stat sb;
 	int fd;
 



CVS commit: src/tests/fs/vfs

2019-09-21 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sat Sep 21 14:25:42 UTC 2019

Modified Files:
src/tests/fs/vfs: t_ro.c

Log Message:
Initialise the sometvs array of struct timeval that is to be used to
validate that utimes() cannot update the times of a file on a read only
filesystem.   The values are never actually used, but since
src/sys/kern/vfs_syscalls.c 1.535
they are validated for sanity, and the syscall returns EINVAL if the
values passed are invalid (tv_usec <0 or >= 100).  If that happens
we don't get as far as the test which produces the EROFS that is expected
from this test (these tests - one for each filesystem type).

So, init the timeval structs (just to 0, the values will still not be
used) so that the EINVAL doesn't bite us before we're eaten by the EROFS
which is the way we're supposed to die.

If the syscall API args were labelled as "const" the compiler probably
would have caught the use of uninit'd vars and complained much sooner.


To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/tests/fs/vfs/t_ro.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/dev/ic

2019-09-21 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sat Sep 21 12:57:25 UTC 2019

Modified Files:
src/sys/dev/ic: mpt.c

Log Message:
bus_addt_t is different widths on different archs, so there is no
one simple %?x format that will always work to print it.  Cast to
intmax_t and use %jx which should work everywhere.


To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.20 src/sys/dev/ic/mpt.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/dev/ic

2019-09-21 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sat Sep 21 12:57:25 UTC 2019

Modified Files:
src/sys/dev/ic: mpt.c

Log Message:
bus_addt_t is different widths on different archs, so there is no
one simple %?x format that will always work to print it.  Cast to
intmax_t and use %jx which should work everywhere.


To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.20 src/sys/dev/ic/mpt.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/ic/mpt.c
diff -u src/sys/dev/ic/mpt.c:1.19 src/sys/dev/ic/mpt.c:1.20
--- src/sys/dev/ic/mpt.c:1.19	Sat Sep 21 07:08:27 2019
+++ src/sys/dev/ic/mpt.c	Sat Sep 21 12:57:25 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: mpt.c,v 1.19 2019/09/21 07:08:27 maxv Exp $	*/
+/*	$NetBSD: mpt.c,v 1.20 2019/09/21 12:57:25 kre Exp $	*/
 
 /*
  * Copyright (c) 2000, 2001 by Greg Ansley
@@ -110,7 +110,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: mpt.c,v 1.19 2019/09/21 07:08:27 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: mpt.c,v 1.20 2019/09/21 12:57:25 kre Exp $");
 
 #include 
 
@@ -327,8 +327,8 @@ mpt_send_cmd(mpt_softc_t *mpt, request_t
 	if (mpt->verbose > 1) {
 		u_int32_t *pReq;
 		pReq = req->req_vbuf;
-		mpt_prt(mpt, "Send Request %d (%#lx):",
-		req->index, req->req_pbuf);
+		mpt_prt(mpt, "Send Request %d (%#jx):",
+		req->index, (intmax_t)req->req_pbuf);
 		mpt_prt(mpt, "%08x %08x %08x %08x",
 		pReq[0], pReq[1], pReq[2], pReq[3]);
 		mpt_prt(mpt, "%08x %08x %08x %08x",



Re: Leak Sanitizer - how to suppress leaks

2019-09-15 Thread Robert Elz
Date:Sun, 15 Sep 2019 19:42:06 +
From:David Holland 
Message-ID:  <20190915194206.gb6...@netbsd.org>

  | 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

Kamil Rytarowski  said:
  | err(3) / abort(3) etc, are abnormal exit. Abnormal one differs from normal/ 
  | clean. For the same reason nobody registers a SIGFPU signal handler to free
  | resources.

doesn't apply, and processes need to clean up after themselves
evern when they abort.

If the system (whatever it is) can handle aborted processes without
suffering, but cannot handle cleanly exiting ones (which would be
bizarre, but never mind) then the simple solution for any process is
simply to always abort:
err(0, "normal exit");

Also remember, that it is not only exit (via normal means, via an error
exit, or even killed by a signal) that requires cleanup, but exec() as
well.

That means, for example, that if you truly believe that a program should
free all of its resources before exiting, then you must also believe that
it must do the same before exec() (as the process is gone after that
happens, and nothing else knows what memory it has or where it was put)
which makes implementations of routines like popen() and system() kind
of interesting to imagine.

This is why the unix environment defines it as a system responsibiliy to
do that cleanup, not the process' and that's the model in which we program.

Expecting any unix program (even the simplest) to simply compile and run
in a non-kernel environment is pointless, they simply won't work.

I used to program such things in the past (distant past) - one of the
requirements of the particular system I was using was that processes
were not allowed to run for "too long" before calling the system process
switch function (no kernel running clock interrupts to do time slicing).

Do we want to make all of our processes able to run in an environment
like that as well?

  | 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?

That isn't the issue at all - in the programs in question, there's no
issue with the lifetimes of objects, it is from creation until something
explicitly makes them go away, or process exit, whichever comes first.

Unlike garbage collection systems, there's no difficulty in finding the
allocated objects, the issue is that there are *lots* of them, in lots
of different data structures, all of which in the "free before exit" model
have to be freed - lots of data structure walking, and not just once, as
these programs fork() a lot, and those child processes inherit all the
allocated memory - it would have to be freed in every one of the children.

And all for no useful purpose, as the system does the free for us when the
process image is deleted.   And we rely upon that everywhere.

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.

kre



Re: Leak Sanitizer - how to suppress leaks

2019-09-13 Thread Robert Elz
Date:Fri, 13 Sep 2019 19:03:40 +0700
From:Robert Elz 
Message-ID:  <24855.1568376...@jinx.noi.kre.to>

  | Even lint had that ability from is beginning (30 years ago).

40.   I cannot subtract...  (and s/is/its/  - I cannot type either!)

kre




Re: Leak Sanitizer - how to suppress leaks

2019-09-13 Thread Robert Elz
Date:Fri, 13 Sep 2019 02:07:43 +0200
From:Kamil Rytarowski 
Message-ID:  <1ea33d0c-fb0f-ecdd-1706-b11841dc6...@gmx.com>

  | Please note that in all arguments about leak sanitization, that it uses
  | internally the same mechanism as garbage collectors. lsan and libgc (and
  | others) also server similar purpose.

Noted.

But please note that all of that means nothing to me at all.   Nor do I
care how any of those things work.   What they do is of some interest,
how they do it is not.

  | LSan can detect indirect leaks:

That's nice.

  | No more leaks were reported, if there are any I will address them.

See my (was very recent when I typed this...) reply to wiz's message.

  | LSan/NetBSD still needs more work and it is not expected to be perfect.

I am not asking for it to be perfect.   What I am requesting is intelligent
interpretation of its results, not just "LSan says this is wrong, so I must
immediately fix it".

  | How about adding ifdef __NO_LEAKS like:
  |
  | #ifdef __NO_LEAKS
  | free(3)?
  | #endif
  |
  | And in lsan/asan/valgrind/etc checks use -D__NO_LEAKS.

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.

The problem more is that we need a way to tell LSan (and the others)
"yes, you told me this looks wrong, I investigated, it is OK, there
is no need to ever tell me about this again".   Even lint had that
ability from is beginning (30 years ago).

With that these tools can actually be useful (when used properly) - when
an issue is reported, if it is a bug, it gets fixed, if it isn't, then
the tool is told that, and we move on.

  | Feel free to experiment.

Right now, I can't - my build system is unable to build anything llvm
related (its compiler is too old) - which is why I haven't been making
any src changes recently.   Currently I can't build the TOOLS needed to
build everything else.   I need to update it so it can actually
build things again...   In progress, but very slowly.

  | I think we need to specify the definition. Leak is a memory object
  | without a pointer referencing it.

I can accept that as a definition.   But we also need to recognise that
there are no leaks after the program has finished.   And the program has
finished as soon as any of exit()/exec*()/_exit() is called (and succeeds)
or when main() returns.   After that there's nothing left of interest.
Leak detection needs to happen before one of those events occurs.

  | A special type of a leak is referencing memory that is no longer needed
  | and could be freed.

That one will be interesting to find...


  | From the compiler point of view it is only slightly different than any
  | regular "int foo(int, char**)".

>From the compiler's viewpoint you're right, main() is just a function.
But from the overall system's viewpoint it isn't.

  | From sanitizer point of view it is not distinguishable and we cannot
  | (easily) make any conditions based on this.

This is a problem, as return from main, and a call to exit() are
equivalent.   Perhaps we should stop returning from main, and always
explicitly call exit() ?

  | Actually globals are not used in LSan and can be ignored.

I don't know how to interpret that.   In many programs the pointers
to allocated memory are stored in (or found from other data stored in)
global vars.

  | Dynamic linker execution time is a part of execution time of the whole
  | application.

Of course.

  | This implementation of __suppress_leak() takes 9-10 amd64 CPU
  | instructions for each __suppress_leak() call.

What does each of those calls accomplish?   That is, how many of
them are needed?

  | Before getting measurable performance impact of __suppress_leak() we
  | will can go out of memory..

It isn't the call itself which is probably the issue, but traversing
all the data structs (some of which may not have been used in ages, and
might be paged out) in order to find all the pointers to the memory that
is allocated.   Alternatively, we could do:

void *
xmalloc(size_t n)
{
void *p = malloc(n);

if (p == NULL) {
err(1, ""); /* or whatever is appropriate *.
}

__suppress_leak(p);

return p;
}

but is that going to help anything at all, ever?

  | In the context of anything executed in libc/csu/application, even for a
  | hello world application this is negligible.

It isn't the trivial applications that matter, it is the big ones.   And it
isn't the comparison with time currently spent, but how much addional time
is consumed, and whether there is any benefit gained from that.

For the ps example, the actual 

Re: Leak Sanitizer - how to suppress leaks

2019-09-12 Thread Robert Elz
Date:Thu, 12 Sep 2019 15:12:25 +0200
From:Kamil Rytarowski 
Message-ID:  <2a6e1fb2-cedc-4a57-750b-45f101be9...@gmx.com>


  | This proposal is practically equivalent of disabling leak detection at
  | all and removes the whole purpose.

No it isn't.   Or rather, it might be, the way you describe LSan to
work, but if that's what it is doing, then it is close to useless:

  | Leak detection works simply by scanning memory (TLS, stacks, heap) for
  | pointers to allocations. If there is no longer a reference, than there
  | is a leak.

So the following code

struct list {
struct list *prev;
struct list *next;
int age;
/* other stuff not relevant here */
};

struct list *
new_list(int age1, int age2)
{
strust list *l1, *l2;

l1 = malloc(sizeof *l1);/* check for NULL elided here */
l2 = malloc(sizeof *l2); /* ditto */

l1->prev = NULL; l1->next = l2; l1->age = age1;
l2->prev = l1; l2->next = NULL; l2->age = age2;

if (age1 > age2)
return NULL;

return l1;
}

has no leak (in the case age1 > age2) ?   Scanning memory would see
a pointer to *l1 (in *l2), and a pointer to *l2 (in *l1).  Both blocks
of memory are referenced according to that trivial analysis.   Useless.

The "close to" just above came from there being cases that it can find.

Because of this, I also looked at the change you pade to "fix" ps.
You added

free(pinfo);

But *pinfo (a struct pinfo) contains 3 pointers, each of which may
refer to other malloc'd blocks.   Further, pinfo points to an array
of those structs.   Because of the memory scan, those pointers (in each
element of the array) will still have been seen, but when you free(pinfo)
then those pointers can no longer be accessed (depending upon what the free()
in use while the sanatiser is running does, they may not still exist
in memory, or they might, just in a block that is now free).  Anyway
the struct kinfo_proc2 struct kinfo_lwp and char * (prefix) entries in
all the struct pinfo's are not being freed.  There was no leak before, but
your "fix" introduced one (or three * N).

I'd suggest reverting the change you made, and instead make pinfo a
global instead of a local in main - then its value will be seen still
existing by the memory scan (LSan will be happy) and ps won't be wasting
(a trivial amount of) time doing an incorrect (as it is now) free(), and
we'll all be happier.

Incidentally, what happens with code like:

struct foo {
int a, b, c;
};

int *
make_foo(int a, int b, int c)
{
struct foo *p;

p = malloc(sizeof *p);  /* again NULL check elided here 
*/
p->a = a; p->b = b; p->c = c;

return (>b);
}

int * global_ptr;

main()
{
global_ptr = make_foo(1,2,3);

/* ... */

{
struct foo *p;

p = (struct foo *)((intptr_t)global_ptr -
 offsetof(struct foo, b));

p->a++;
}

/* ... */
}

Ugly (and stupid) but not illegal I think.   The more common case is
a list where the back pointers point at the forward pointer, rather
than at the struct itself, so one can do

if ((*(p->back) = p->next) != NULL)
p->next->back = p->back
free(p);

to delete an element from the middle of a list easily - which works
even if the element to be deleted is first or last.   There if the
next field is the first in the struct, the back field will refer to
the malloc's memory block (by accident) but if it isn't, it won't.

  | I have not investigated where are the sh(1) leaks,

As far as I know it doesn't.   What I said is that it does not free
memory it allocates before it exits.  That is not a leak.

  | In sanitizers we cannot overload or do anything special with main()
  | (such as replace it with our own copy).

That wasn't the suggestion.   However, what was assumed a more rational
leak detection algorithm than appears to be in use - I agree, by simply
scanning memory you cannot do anything special with main() as when the
code runs, apparently, main's stack frame has already gone.

But were this done a different way -- the right way is to annotate, in the
code, all assignments to pointers , and add a ref counter to all allocated
memory blocks - when a pointer is assigned, and it points to something
allocated, then incr the ref count.  Whan the value of a pointer is
overwritten, and it used to point to allocated memory, decr the ref count.
If that decrease drops it to 0, then memory has been leaked.  In the
exit code for every 

Re: Leak Sanitizer - how to suppress leaks

2019-09-12 Thread Robert Elz
Date:Thu, 12 Sep 2019 02:58:41 +0200
From:Kamil Rytarowski 
Message-ID:  <373b9331-5306-9797-b4bd-8f6c52683...@gmx.com>

  | I have tested interactive sh(1) with LSan and it does not leak when
  | used.

It doesn't matter what code (if any) you run, there is always memory
allocated in sh that isn't freed when it exits.   Always.   What varies
depending upon what code the shell runs is just how much of it exists.

What this tells me is that the sanatisers aren't detecting unfreed memory
when it is done the way the shell does it.   There are two potential
differences from ps that may be it (perhaps one of them, perhaps both are
needed) - all the shell's allocated memory is referred to from globals,
the variable in question in ps was a local in main().   And second, ps
exits by returning from main(), the shell never does that, it exits via
an explicit call to exit() (or _exit() in one or two cases - the shell forks
a lot so there are several different exit paths depending upon just what
is happening .. all of the forks inherit all the unfreed memory of the
parent shell).

To me it seems apparent that the sanatiser is detecting the local variable
in main() go out of scope when main() returns, and so the value it contains
(the pointer to the allocated memory) is lost, and so it is determined that
there is a leak.   For any other function that would often be true, but for
main() it never is.

If this is what is happening, it would seem that a relatively cost free
fix, instead of calling free() would be simply to make the variable (psinfo)
a global instead of local to main.   That would be a pity, as it makes it
harder to be certain that other code is not fiddling with its value, but
might make this problem go away.

Alternatively, perhaps ps could call exit(eval); instead of doing
return eval; and since one presumes that the sanitisers know that exit()
never returns, then the local variable would never go out of scope, and
hence its memory would not be lost.

The right change to make however is to teach the santitsers (all of them)
that exiting (via exit() or a return from main()) frees everything, and
not to complain about any unfreed memory when that happens (and that local
variables in main effectively have the same lifetime as globals).
This could be an option, so those programs that don't intend to leave
any unfreed memory when they exit could verify that (ideally via a
pragma or magic comment of some kind in the code, rather than yet another
command line option).


  | There is a cost of calling a dummy function _suppress_leak(), but it is
  | probably negligible.

No, it isn't.   The way you have described it, that function needs to be
called for every block of allocated memory that has been allocated, and is
never going to be freed.   That means either calling this function immediately
after all (or much) memory is allocated, even if it turns out that memory is
later freed, as often the code does not know what allocations will be released
and which will still be active when the program terminates.   Or every time
the program is about to exit, it needs to traverse all of its data structures
calling this function on all memory that had perviously been allocated.
Potentially thousands of these dummy function calls.

For something like sh we are not talking about just when the script (or
interactive shell) exits when it is finished, but every time one of its
forked children exits, or calls one of the exec*() family - which also is
effectively exiting the shell (if memory needs to be freed before an exit
it needs to be freed before an exec() as well ... which is tricky when the
args being given to exec() are in malloc'd memory!)

Doing it this way (anything like this way) is simply not feasible.  If there
is to be some new sanatiser specific function call, it needs to be more like
_all_memory_is_free() which simply tells the sanitiser that anything it thinks
is still allocated, should be treated as if it has been freed.

But even that is the wrong way, the right way is to invert that function,
and make it be something like _verify_no_unfreed_memory();

That's a better method than the pragma/magic comment I suggested above to
allow the sanatisers to be told to go check that there is no unallocated
memory, and what's more, can be called any time at all (of course programs
need to be aware that libc functions, like stdio, may have allocated mem,
like FILE buffers, that have not been freed, so use would need to be
cautious).   This still requires the sanatisers to be taught that simply
exiting with unfreed memory is not a problem.

kre



Re: CVS commit: src/bin/ps

2019-09-11 Thread Robert Elz
Date:Wed, 11 Sep 2019 21:13:24 +0200
From:Kamil Rytarowski 
Message-ID:  <6c853bc7-6510-459e-d451-51f988617...@gmx.com>

  | We have got even fixups in libc for such "nonsense" cases.

Why?   In 99% (or more) of libc the fixes are relevant, as those functions
can be called over & over and so should not be discarding memory.

atexit() is a somewhat special case - if simply calling it leaks, then that
ought be fixed, if the "leak" is just the data struct used to keep track
of what needs to be run when the program ends, then not freeing that
stuff is 100% harmless, and there's no point "fixing" it.

  | A workaround would be  ...

I don't know anything about the sanatisers so can't comment on what
should be done with them.

But if you were ever to test sh (and I would guess not just /bn/sh, but
any shell) for this kind of thing, you'd find that at exit none of the
user's (or shell's) variables have been freed, the hash table that
associates command names with the located paths is not freed, aliases are
not freed, nor defined functions, shell history, key bindings for libedit,
defined traps, ... and probably much more I cannot remember just now,
including (if the shell exits via a call to the exit builtin somewhere
in the script) the internal code tree that is currently being evaluated.

It would be a lot of code to go free all of that, it would slow down
shell exit, and all for no benefit at all, as when _exit() eventually
gets called, everything is returned to the kernel, nothing is lost.

Further, other than that the shell is exiting, the data structs in
question all need to be retained, so nothing in them can be freed any time
before the shell exits (obviously other than when that data is being
updated, and the old data is freed).

Does anyone really want to make the shell, or other programs, run slower
just so someone can say that all memory was nicely (but pointlessly) freed
before exit ?

kre



Re: CVS commit: src/bin/ps

2019-09-11 Thread Robert Elz
Date:Wed, 11 Sep 2019 17:02:53 +
From:"Kamil Rytarowski" 
Message-ID:  <20190911170253.d097ff...@cvs.netbsd.org>

  | Free it when no longer used, just before the program termination.

Can we please avoid this kind of nonsense.   Everything is freed when
every program exits - doing explicit free() calls makes the program bigger
and slower for no reason at all.

By all means fix places where memory is truly leaked (whenre more is
continuously allocated, and simply discarded) but anything that is supposed
to remain until program exit should simply go away when the exit happens.

If the canitisers cannot be instructed to ignore such things, they are
much less useful tan they could be.

kre



Re: CVS commit: src

2019-09-03 Thread Robert Elz
Date:Tue, 3 Sep 2019 11:30:03 +0100
From:Sevan Janiyan 
Message-ID:  

  | That would break support for building on macos

Yes, I guessed that was the incentive for keeping it, but doesn't
macos have the ability to turn that off?   Is it really such a huge
imposition to expect people building NetBSD on MacOS to put the
NetBSD sources (and objects, destdir, ...) on filesystems mounted
without that?

kre



Re: CVS commit: src

2019-09-02 Thread Robert Elz
Date:Tue, 3 Sep 2019 04:07:16 +
From:Taylor R Campbell 
Message-ID:  <20190903040716.a6abe60...@jupiter.mumble.net>

  | How do we clean it up?

  | I am not seeing a good way out of this.

I do, but you are all refusing to permit it ... simply abandon support
for case insensitive filesystems.

In practice (and I admit to not really having studied any of them, as
they're all a stupid idea) I doubt that any of them really are truly
case insensitive ... rather than are insenstive to the case of ascii
chars, and that's usually it.

Are there any of these filesystems that treat capital delta the same
as lower case delta?   Or even capital U with an umlaut the same as
lower case u with an umlaut?   (or E-acute and e-acute)   etc?

And if they do, how do they manage to do that?   The only way I can
imagine is to permit only 10646 (however encoded) characters in file
names, but if the file system is like that, how do we support it when
we have files on FFS (which has no such restriction) with characters
with the top bit set?   Is such a character 8859-1, 8859-7, 8859-11?

Those 3 have different methods of mapping their upper case chars to
their lower case equivs as I understand it (for 8859-11 it is easy,
there is no case, so none of the chars in the 0x80-0xff range are
equiv to each other regardless of whether the filesys is case insensitive
or not, but that would not, should not, be true for the other two
mentioned.)

Since none of the sys calls that end up invoking namei() pass locale
info to the kernel, and even if they did, I doubt we'd want to build
case mapping locale specific tables into the kernel (anyone's kernel)
and since I see no justification at all for doing special case handing
of English but not for all other languages, I'd suggest that any unix
filesystem that doesn't treat all filenames as simply bit strings, with
special handling only for the bytes 0x2F and 0x00 is simply broken, and
we should not attempt to support that at all.

Dealing with this kind of thing belongs in the applications, where locale
info is available (or can be), and where what is appropriate for treating
two different strings as if they represent the same thing can be dealt
with in a rational way - at least rational for that application.

So, to me, the solution is simple.   Leave it all as it is now, and
simply tell people using "case insensitive" file systems to either stop
doing that, or they simply will not be able to build everything.

kre



Re: CVS commit: src/sys/kern

2019-08-19 Thread Robert Elz
Date:Mon, 19 Aug 2019 05:32:42 -0400
From:"Christos Zoulas" 
Message-ID:  <20190819093242.88152f...@cvs.netbsd.org>

  | Log Message:
  | If we could not start extattr for some reason, don't advertise extattr
  | in the mount.

I would have expected a better result would be that if an attempt is
made to mount with extattr turned on, and extattr is not available, then
the mount would fail, rather than succeeding with exattr missing.

kre



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

2019-07-22 Thread Robert Elz
Date:Mon, 22 Jul 2019 12:23:35 +0200
From:Tobias Nygren 
Message-ID:  <20190722122335.55c0d421a18a082c8d67b...@netbsd.org>

  | Please exercise caution when changing established userland behaviour.
  | This change broke the pkgsrc/lang/openjdk8(1) build.

I stand behind my previous reply, if that change seemed to break
anything, then all it was really doing was revealing previous brokenness.

However, as well as saying that printf(1) has no options, and (by omission)
also saying that it should not do getopt processing, POSIX also says
that if the format arg (ie: the first arg) contains no % cpnversions,
and yet there are more args (which more or less by defnition can never
be used) then the results are unspecified.

If what is, I am guessing, the "established userland behaviour" was
printf -- format args
which is truly pointless  (and incoorrect), but used to be accepted,
then we can continue to allow that to work, relying upon that
unspecified results clause from POSIX, "--" contains no % conversion,
yet there are more args that nothing would normally ever consume
(technically that format string is supposed to be used over and over
again until the args are all used up ... but obviously that would never
happen in this case - which is why it leads to unspecified results).

I intend to commit a change to printf in a few minutes to make it work
like this (with the caveat that the "no % conversions" test will be "no
% characters" which guarantees no % conversions of course, but is
simpler).

If there is only one arg, it will always be treated as a format, so
printf ---\\n
will continue working as it should, or if the arg contains a % char,
then it will also be treated as a format, so
printf -%d\\n 3
will print -3 as it should (neither of those wworked until recently).

This doesn't mean that whatever script in the openjdk8 build was using
printf incorrectly shouldn't be fixed ... it should still be.

kre



CVS commit: src/usr.bin/printf

2019-07-22 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Mon Jul 22 17:34:31 UTC 2019

Modified Files:
src/usr.bin/printf: printf.c

Log Message:
Amend the previous change: we can have (almost) the best of both
worlds, as when the first arg (which should be the format) contains
no % conversions, and there are more args, the results are unspecified
(according to POSIX).

We can use this so the previous usage
printf -- format arg...
(which is stupid, and pointless, but used to work) continues to
simply ignore the -- (unspecified results mean we can do whatever
feels good...)

This brings back the #if 0'd block from the previous modification
(so there is no longer anything that needs cleaning up later) but runs
the getopt() loop it contained only when there are at least 2 args
(so any 1 arg printf always uses that arg as the format string,
whatever it contains, including just "--") and also only when the
first (format) arg contains no '%' characters (which guarantees no %
conversions without needing to actually parse the arg).  This is the
(or a) "unspecified results" case from POSIX, so we are free to do
anything we like - including assuming that we might have options
(we don't) and pretending to process them.


To generate a diff of this commit:
cvs rdiff -u -r1.49 -r1.50 src/usr.bin/printf/printf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/printf/printf.c
diff -u src/usr.bin/printf/printf.c:1.49 src/usr.bin/printf/printf.c:1.50
--- src/usr.bin/printf/printf.c:1.49	Sun Jul 21 15:25:39 2019
+++ src/usr.bin/printf/printf.c	Mon Jul 22 17:34:31 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: printf.c,v 1.49 2019/07/21 15:25:39 kre Exp $	*/
+/*	$NetBSD: printf.c,v 1.50 2019/07/22 17:34:31 kre Exp $	*/
 
 /*
  * Copyright (c) 1989, 1993
@@ -41,7 +41,7 @@ __COPYRIGHT("@(#) Copyright (c) 1989, 19
 #if 0
 static char sccsid[] = "@(#)printf.c	8.2 (Berkeley) 3/22/95";
 #else
-__RCSID("$NetBSD: printf.c,v 1.49 2019/07/21 15:25:39 kre Exp $");
+__RCSID("$NetBSD: printf.c,v 1.50 2019/07/22 17:34:31 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -138,27 +138,39 @@ main(int argc, char *argv[])
 
 	rval = 0;	/* clear for builtin versions (avoid holdover) */
 
-#if 0
-	int o;
-
 	/*
 	 * printf does not comply with Posix XBD 12.2 - there are no opts,
 	 * not even the -- end of options marker.   Do not run getoot().
 	 */
-	while ((o = getopt(argc, argv, "")) != -1) {
-		switch (o) {
-		case '?':
-		default:
-			usage();
-			return 1;
+	if (argc > 2 && strchr(argv[1], '%') == NULL) {
+		int o;
+
+		/*
+		 * except that if there are multiple args and
+		 * the first (the nominal format) contains no '%'
+		 * conversions (which we will approximate as no '%'
+		 * characters at all, conversions or not) then the
+		 * results are unspecified, and we can do what we
+		 * like.   So in that case, for some backward compat
+		 * to scripts which (stupidly) do:
+		 *	printf -- format args
+		 * process this case the old way.
+		 */
+
+		while ((o = getopt(argc, argv, "")) != -1) {
+			switch (o) {
+			case '?':
+			default:
+usage();
+return 1;
+			}
 		}
+		argc -= optind;
+		argv += optind;
+	} else {
+		argc -= 1;	/* drop argv[0] (the program name) */
+		argv += 1;
 	}
-	argc -= optind;
-	argv += optind;
-#else
-	argc -= 1;
-	argv += 1;
-#endif
 
 	if (argc < 1) {
 		usage();



CVS commit: src/usr.bin/printf

2019-07-22 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Mon Jul 22 17:34:31 UTC 2019

Modified Files:
src/usr.bin/printf: printf.c

Log Message:
Amend the previous change: we can have (almost) the best of both
worlds, as when the first arg (which should be the format) contains
no % conversions, and there are more args, the results are unspecified
(according to POSIX).

We can use this so the previous usage
printf -- format arg...
(which is stupid, and pointless, but used to work) continues to
simply ignore the -- (unspecified results mean we can do whatever
feels good...)

This brings back the #if 0'd block from the previous modification
(so there is no longer anything that needs cleaning up later) but runs
the getopt() loop it contained only when there are at least 2 args
(so any 1 arg printf always uses that arg as the format string,
whatever it contains, including just "--") and also only when the
first (format) arg contains no '%' characters (which guarantees no %
conversions without needing to actually parse the arg).  This is the
(or a) "unspecified results" case from POSIX, so we are free to do
anything we like - including assuming that we might have options
(we don't) and pretending to process them.


To generate a diff of this commit:
cvs rdiff -u -r1.49 -r1.50 src/usr.bin/printf/printf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



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

2019-07-22 Thread Robert Elz
Date:Mon, 22 Jul 2019 12:23:35 +0200
From:Tobias Nygren 
Message-ID:  <20190722122335.55c0d421a18a082c8d67b...@netbsd.org>

  | This change broke the pkgsrc/lang/openjdk8(1) build.

That it is broken and ought to be patched upstream, it is quite
clear that printf is required to take no options at all - there was
a bug report about it not correctly handling "printf ---" (actually with
a newline appended, but that is irrelevant) which it was correct to fix.

Sometimes stuff that has relied upon bugs simply breaks when the bugs
get fixed.

kre




CVS commit: src/tests/usr.bin/printf

2019-07-21 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sun Jul 21 15:25:59 UTC 2019

Modified Files:
src/tests/usr.bin/printf: printf.sh

Log Message:
Stop assuming that printf handles options in any way at all
(it doesn't - that is, shouldn't) which includes processing -- as an
"end of options".  The first arg is (always) the format string.

Remove/fix tests that assumed the contrary.

Problem (with printf) pointed out on tech-userlevel by Thierry Laronde.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/tests/usr.bin/printf/printf.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/tests/usr.bin/printf/printf.sh
diff -u src/tests/usr.bin/printf/printf.sh:1.3 src/tests/usr.bin/printf/printf.sh:1.4
--- src/tests/usr.bin/printf/printf.sh:1.3	Fri Sep 14 19:57:57 2018
+++ src/tests/usr.bin/printf/printf.sh	Sun Jul 21 15:25:59 2019
@@ -1,4 +1,4 @@
-# $NetBSD: printf.sh,v 1.3 2018/09/14 19:57:57 kre Exp $
+# $NetBSD: printf.sh,v 1.4 2019/07/21 15:25:59 kre Exp $
 #
 # Copyright (c) 2018 The NetBSD Foundation, Inc.
 # All rights reserved.
@@ -206,23 +206,21 @@ basic()
 {
 	setmsg basic
 
-	for A in '' -- -@	# hope that '@' is not an option to printf...
-	do
-		if (do_printf $A >/dev/null 2>&1)
-		then
-			atf_fail "${A:-with no args} successful"
-		fi
-		if test -n "$( do_printf 2>/dev/null )"
-		then
-			atf_fail "${A:-with no args} produces text on stdout"
-		fi
-		if test -z "$( do_printf 2>&1 )"
-		then
-			atf_fail "${A:-with no args} no err/usage message"
-		fi
-
-		test -z "$A" && continue
+	if (do_printf >/dev/null 2>&1)
+	then
+		atf_fail "with no args successful"
+	fi
+	if test -n "$( do_printf 2>/dev/null )"
+	then
+		atf_fail "with no args produces text on stdout"
+	fi
+	if test -z "$( do_printf 2>&1 )"
+	then
+		atf_fail "with no args no err/usage message"
+	fi
 
+	for A in - -- X 1
+	do
 		if (do_printf "%${A}%" >/dev/null 2>&1)
 		then
 			atf_fail "%${A}% successful"
@@ -232,11 +230,10 @@ basic()
 	expect abcd		abcd
 	expect %		%%
 	expect xxx%yyy		xxx%%yyy
-	expect -123		-- -123
+	expect -123		-123
 
 	# technically these are all unspecified, but the only rational thing
 	expect_fail ''		%3%
-	expect_fail ''		-123
 	expect_fail a		a%.%
 	expect_fail ''		'%*%b'	# cannot continue after bad format
 	expect_fail a		a%-%b	# hence 'b' is not part of output
@@ -1126,7 +1123,7 @@ G_floats()
 }
 define G_floats '%G (floating) conversions'
 
-# It is difficul;t to test correct results from the %a conversions,
+# It is difficult to test correct results from the %a conversions,
 # as they depend upon the underlying floating point format (not
 # necessarily IEEE) and other factors chosen by the implementation,
 # eg: the (floating) number 1 could be 0x8p-3 0x4p-2 0x1p-1 even



CVS commit: src/tests/usr.bin/printf

2019-07-21 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Sun Jul 21 15:25:59 UTC 2019

Modified Files:
src/tests/usr.bin/printf: printf.sh

Log Message:
Stop assuming that printf handles options in any way at all
(it doesn't - that is, shouldn't) which includes processing -- as an
"end of options".  The first arg is (always) the format string.

Remove/fix tests that assumed the contrary.

Problem (with printf) pointed out on tech-userlevel by Thierry Laronde.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/tests/usr.bin/printf/printf.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



  1   2   3   4   >