Re: CVS commit: src/sbin/gpt

2017-02-13 Thread David Laight
On Sun, Feb 12, 2017 at 06:20:53PM +, Christos Zoulas wrote:
> In article <87bmu7l1k6@free.fr>,
> Aymeric Vincent  <aymericvinc...@free.fr> wrote:
> >chris...@astron.com (Christos Zoulas) writes:
> >
> >> Yes, doesn't libcompat provide our getopt()? Or are you trying to compile
> >> this standalone?
> >
> >It does, but from what I understand, we replace only getopt_long()
> >unconditionnally and use the host's getopt() if the optind variable can
> >be linked against.
> >
> >Confirmed on a Linux host in /lib:
> >
> >$ nm libnbcompat.a | grep getopt
> >getopt_long.lo:
> >0098 T __nbcompat_getopt_long
> >$ 
> >
> >I agree it would be nicer to use always our getopt().
> 
> Yes, because the glibc is not posix compliant by default.

More than non-compliaint, completely f*cked.
It reorders argv[] to move all 'options' before filenames.
So 'foo bar -baz' is changed to 'foo -baz bar' before being processed.
I've NFI of the justification for it.
Historically you could do 'rlogin host -l username' but I
don't know of any others.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/net

2016-01-12 Thread David Laight
On Fri, Dec 11, 2015 at 11:37:29AM +0900, Kengo NAKAHARA wrote:
> # Sorry, I forgot to subscribe source-changes-d ml, I reply as
> # a new mail.
> 
> Hi,
> 
> > In article <20151210081103.E0FBBFB83%cvs.NetBSD.org@localhost>,
> > Kengo NAKAHARA <source-changes-d%NetBSD.org@localhost> wrote:
> >>-=-=-=-=-=-
> >>
> >>Module Name:src
> >>Committed By:   knakahara
> >>Date:   Thu Dec 10 08:11:03 UTC 2015
> >>
> >>Modified Files:
> >>src/sys/net: if_gif.c
> >>
> >>Log Message:
> >>kmem_zalloc(, KM_SLEEP) must not return NULL.
> > 
> > I would like to solicit opinions about this change and form a general
> > policy.
> > 
> > 1. I would like to reduce the use of KASSERT in the kernel, specially
> > in situations like thee above where the test can be centralized (inside
> > kmem_alloc) and avoided without being fatal.
> 
> OK, this kmem_zalloc() is not fatal. I should avoid KASSERT here.

There is no real point using KASSERT() when the immediate
dereference of NULL will have the same effect and is about as
easy to debug.

Whether kmem_alloc(KM_SLEEP) can return NULL is another matter.
Seems wrong to me - maybe even for impossible allications.
ISTR a problem waiting for KVA and phys-mem being 'difficult'.
I know that the Linux equivalent will return NULL, not sure when.
It would be useful to define that allocation for non-huge
(maybe even several MB) amounts will always sleep.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/sh

2016-01-12 Thread David Laight
On Tue, Jan 05, 2016 at 01:19:36AM +, Taylor R Campbell wrote:
>Date: Mon, 4 Jan 2016 20:06:55 -0500
>From: chris...@zoulas.com (Christos Zoulas)
> 
>On Jan 5,  5:33am, k...@munnari.oz.au (Robert Elz) wrote:
>-- Subject: Re: CVS commit: src/bin/sh
> 
>|   | Does the exec'ed program know what to do with fd > 2?
>|   | Is it hard-coded, or do we specify it with -fd N?
>| 
>| More likely, if this ever was to be used, it would be /dev/fd/N
>| but certainly this is not going to be common.
> 
>Right, otherwise people would complain a lot more about ksh than
>they currently do looking at the web...
> 
> I'm pretty sure I have shell scripts that rely on /dev/fd/N working as
> an exec'd command argument for N > 2.  An example of an exec'd program
> that uses N > 2 without /dev/fd/N is gpg, with the --passphrase-fd,
> --command-fd, --status-fd options.  I would be unhappy if these broke.
> 
> ...unless I've misunderstood what this thread is about from my very
> cursory skimming of it.

Same here.
I'm pretty sure I've passed /dev/fd/3 as a command line paramter
(although you often want multiple pipes - which is too hard).

I remember some shells closing everything except 0, 1 and 2 on entry
(perhaps unless/if some command line option given).
But that is different.

I'll use 'exec n>file' (etc) to get the shell to access an extra
file (or 2 or more). Although they are probably dup'ed back before
anything is run.

Personally I think it is the job of the scripts to close
any extra fd, not the shell.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS import: othersrc/external/bsd/ibbs

2016-01-12 Thread David Laight
On Mon, Nov 23, 2015 at 05:56:01AM +, Alistair G. Crooks wrote:
> Module Name:  othersrc
> Committed By: agc
> Date: Mon Nov 23 05:56:01 UTC 2015
> 
> Update of /cvsroot/othersrc/external/bsd/ibbs
> In directory ivanova.netbsd.org:/tmp/cvs-serv12751
> 
> Log Message:
> Import an integer-based version of the Blum Blum Shub random number
> generator into othersrc.
> 
>   IBBS - Integer Blum Blum Shub Random Number Generator
>   =
> 
>   This is a small Blum Blum Shub implementation which uses a Mersenne
>   Twister to take 4 bytes of entropy (retrieved from the microseconds
>   part of gettimeofday(2)), and generates 2 prime numbers and a seed from
>   this.  Each prime number and seed is 16 bits.  A deterministic prime
>   check is used to ensure we are dealing with safe/unsafe prime numbers.
> 
>   Since 16 bits are used for the two primes, care is taken to avoid
>   cycles in the BBS output. If a cycle is detected, the generator is
>   re-seeded, and output starts again.
> 
>   The RNG seems to be quite efficient, generating numbers at 10 MBps
>   on a NetBSD VM running in Fusion hosted on Mac OS X.

Doesn't sound like anywhere near enough entropy.
If you start with 32 bits you'll get 'birthday paradox' duplicated
sequences after a relatively small number of boots.

David

-- 
David Laight: da...@l8s.co.uk


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

2016-01-12 Thread David Laight
On Sat, Nov 14, 2015 at 06:40:21AM +1100, matthew green wrote:
> Christos Zoulas writes:
> > In article <2015111344.ga13...@britannica.bec.de>,
> > Joerg Sonnenberger  <jo...@britannica.bec.de> wrote:
> > >On Thu, Nov 12, 2015 at 12:23:51PM -0500, Christos Zoulas wrote:
> > >> Module Name: src
> > >> Committed By:christos
> > >> Date:Thu Nov 12 17:23:51 UTC 2015
> > >> 
> > >> Modified Files:
> > >>  src/common/lib/libc/stdlib: _strtol.h _strtoul.h
> > >> 
> > >> Log Message:
> > >> Recognize 0[bB] as binary (base 2)
> > >
> > >Based on what authority? This is a ISO C function and that doesn't allow
> > >binary input. I am quite concerned about changing a function used that
> > >often, especially as it can break a lot of existing code.
> > 
> > I don't think it will since it will only affect conversions with 0[bB],
> > and the OS/X code is doing the same, but I will revert it until others
> > catch up.
> 
> the problem is that something that was "0b" always came out
> as 0 before, but now it doesn't.
> 
> that's a fairly major semantic change, i think i agree with joerg that
> it has a high chance of breaking existing usage.

Worse that that, some code might be relying on getting a pointer
to the 'b' and continuing to parse the buffer.

Not a good idea to change it.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand/bootxx

2016-01-12 Thread David Laight
On Mon, Jan 04, 2016 at 06:33:50PM +1100, matthew green wrote:
> "Christos Zoulas" writes:
> > Module Name:src
> > Committed By:   christos
> > Date:   Sun Jan  3 20:59:47 UTC 2016
> > 
> > Modified Files:
> > src/sys/arch/i386/stand/bootxx: pbr.S
> > 
> > Log Message:
> > change 60 to 70 which is the current release. Noticed by Rares Aioanei.
> 
> i don't think so we should do this unless we change the protocol,
> as jak mentioned.  this can be intepreted by the BIOS and it might
> do differnet stuff so unless there's actually a good reason for it
> this change could break something that works (poorly.)

AFAIR it is just the operating system name in the partition
boot record and is purely a comment.

The bios certainly should be looking at it, the bios doesn't
look at the pbr only the mbr (and shouldn't look at much of that).

OTOH, no point changing it.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2015-10-18 Thread David Laight
On Tue, Oct 13, 2015 at 09:28:35PM +, Robert Swindells wrote:
> Module Name:  src
> Committed By: rjs
> Date: Tue Oct 13 21:28:35 UTC 2015
... 
> Log Message:
> Add core networking support for SCTP.

As a matter of interest, where did this come from?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2015-09-27 Thread David Laight
On Thu, Sep 24, 2015 at 10:30:52AM -0400, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Thu Sep 24 14:30:52 UTC 2015
> 
> Modified Files:
>   src/sys/kern: makesyscalls.sh
> 
> Log Message:
> create an array of altsyscallnames, which are the syscall names that the
> libc api uses. For example syscallnames[293] = "__sigprocmask14",
> altsyscallnames[293] = "sigprocmask". This is so that things like dtrace
> can use the system call names everyone uses. The array is sparse; if the
> names were the same (or for compat names) they are not copied and the array
> has NULL for them.

H
Isn't that going to be even more confusing since you won't know
whether a 'compat' (or old) system call is being used which might
have issues in the compatibility layer itself.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/m68k/fpsp

2015-09-06 Thread David Laight
On Sat, Sep 05, 2015 at 09:39:43AM +0900, Masao Uebayashi wrote:
> The reason of this is that config(1) have no idea of library at this
> moment.  Makefile.kern.inc has also a convention that all *.o files
> have to be built under the top of kernel build directory.  libkern &
> libcompat have speicalized make(1) rules, that work but look ugly.
> I'd consider to extend config(1) to support library...

Please can you keep some context in your emails.
The 'this' is the first line is a reference to something unknown.

Oh, and one historic reason for the kernel objects being in a .a
file was to allow builds on systems with short command line length
limits.

    David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2015-09-02 Thread David Laight
On Wed, Sep 02, 2015 at 04:30:27PM +, Quentin Garnier wrote:
> On Wed, Sep 02, 2015 at 03:49:22PM +, Christos Zoulas wrote:
> > In article <20150902134214.d867...@cvs.netbsd.org>,
> > Masao Uebayashi <source-changes-d@NetBSD.org> wrote:
> > 
> > -#define CONFIG_VERSION 20150840
> > +#define CONFIG_VERSION 20150841
> > 
> > This was supposed to be a date...
> 
> When I introduced it my DNS-zone-file-encumbered head though of adding
> a couple 0s there to handle multiple versions within a day, but then I
> thought about it again and came to the conclusion that if I felt I
> needed to bump that number more than once in a day, it would mean that
> I'm not designing and/or developing correctly.  I feel that conclusion
> still stands.

If desparate you can always use tomorrow...

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/gpl3

2015-08-31 Thread David Laight
On Tue, Aug 11, 2015 at 10:26:43PM +0400, Alan Barrett wrote:
> On Tue, 11 Aug 2015, David Laight wrote:
> >The system should probably clean 'turds' from both /tmp and /var/tmp.
> 
> That would be surprising, at least to me.
> 
> I rely on /var/tmp/vi.recover being persistent, and I often
> use /var/tmp as a place to stash work in progress that should survive a
> reboot.

I was thinking of stuff that is old, not deleting everything on reboot.

David

-- 
David Laight: da...@l8s.co.uk


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

2015-08-31 Thread David Laight
On Sat, Aug 29, 2015 at 12:03:51PM +0900, Masao Uebayashi wrote:
> Such a hack is needed because config(1) has to generate rules
> explicitly for each *.[cS].  If you try to override a rule (e.g.
> compile this pmap_bootstrap.c with ${NOPROF_C}), it will be a
> duplicated rule.
> 
> If *.[cS] -> *.o will be written using suffix rules, you can safely
> override rules.  No order constraint too.

You may want to define an explicit dependency foo.o: foo.[cS] and then
rely on the suffix rule to supply the commands.
This saves make from having to hunt for the appropriate source file
and (probably) makes it easier to have MI fallback sources.

    David

-- 
David Laight: da...@l8s.co.uk


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

2015-08-11 Thread David Laight
On Thu, Jul 30, 2015 at 05:47:51PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Jul 30 21:47:51 UTC 2015
 
 Modified Files:
   src/usr.bin/patch: pch.c
 
 Log Message:
 from bitrieg:
 
 Substitution commands might contain a newline in the replacement pattern
 (escaped with a backslash before it), causing patch's understanding of
 the state the ed child process is in to diverge from reality. This can
 lead to patch unwillingly feeding '!' (execute shell command) lines to
 ed. Finding out how to do this is left as an exercise to the reader.
 
 XXX: pullup-7

Maybe patch should stop ed from executing shell commands.
Setting SHELL=/bin/false might be enough.

David

-- 
David Laight: da...@l8s.co.uk


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

2015-08-11 Thread David Laight
On Mon, Aug 10, 2015 at 05:12:54PM +, David Holland wrote:
 On Sun, Aug 09, 2015 at 01:06:30AM +0200, Aymeric Vincent wrote:
   David Holland dholland-sourcechan...@netbsd.org writes:
   
  Fix bracket expressions by moving '-' to the end of them. GNU awk 
 choked.
   
Front is safer. fwiw.
   
   OK, I moved them to the front, together with '_' because it felt awkward
   to separate the separators. Out of curiosity, is it safer just because
   it is more robust to future additions to the expression or is it
   actually safer even if the expression is left as is?
 
 It's safer in the sense that a broken regexp parser is unlikely to
 accidentally treat the first character of a character set as the
 middle - in a range; but if the last character is - it might interpret
 the second-last character, the -, and closing ] as a range, with
 unfortunate results. It is not very likely; but stranger things have
 happened, and it doesn't help that regexp tools have a long-standing
 culture of not complaining about invalid regexp syntax.
 
 anyway it's a very minor point.

It is also less likely to go wrong when someone adds another character.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/gpl3

2015-08-11 Thread David Laight
On Mon, Aug 10, 2015 at 03:45:41PM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Mon Aug 10 15:45:40 UTC 2015
 
 Modified Files:
   src/external/gpl3/binutils/dist/libiberty: make-temp-file.c
   src/external/gpl3/gcc/dist/libiberty: make-temp-file.c
   src/external/gpl3/gdb/dist/libiberty: make-temp-file.c
 
 Log Message:
 Correct temporary directory preference order in libiberty's choose_tmpdir().
 
 Because it is intended to be persistent, /var/tmp is about the worst possible
 choice for temporary files for most users of libiberty.  /tmp works better,
 because the the defined semantics of /tmp allow for a non-persistent tmpfs
 to be used.  This should improve performance when /tmp is a tmpfs and it is
 difficult or impossible to have an environment variable or command line -pipe
 flag passed to every piece of the toolchain.

Historically /tmp was likely to be small.
That is why /var/tmp exists, and is probably why the compiler defaults
to /var/tmp.
The system should probably clean 'turds' from both /tmp and /var/tmp.

David

-- 
David Laight: da...@l8s.co.uk


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

2015-07-07 Thread David Laight
On Wed, Jul 01, 2015 at 02:04:43AM +, Christos Zoulas wrote:
 In article 20150630233112.ga8...@britannica.bec.de,
 Joerg Sonnenberger  jo...@britannica.bec.de wrote:
 On Tue, Jun 30, 2015 at 05:08:24PM -0400, Christos Zoulas wrote:
  Module Name:   src
  Committed By:  christos
  Date:  Tue Jun 30 21:08:24 UTC 2015
  
  Modified Files:
 src/sys/arch/amd64/amd64: cpu_in_cksum.S
  
  Log Message:
  handle PIC compilation (if we are building a PIE system; this is used
 by tests)
 
 Isn't the leaq generally preferable as smaller?
 
 I believe leaq is 7 bytes, and movq is 5. But I am not sure which takes
 more cycles.

'leaq' with %rip will have to be a long encoding since rip relative
isn't a 386 addressing mode.

My guess is that both take the same number of cycles on current cpus.
Some old brain cells recall 'lea' using different hardware from the ALU
(for adds) so happening at a different stage in the pipeline and having
different result delay and/or concurrency rules - but I can't remember
which particular cpu that applied to.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2015-03-14 Thread David Laight
On Wed, Mar 11, 2015 at 06:12:32PM -0400, Greg Troxel wrote:
 
 David Laight da...@l8s.co.uk writes:
 
  I'm also not sure we need a new kernel config.  It seems the systems of
  interest are limited to 486 machines without PCI (and thus EISA
  probably), and that's a pretty narrow window around 1993-1994.  So
  leaving the lines commented out with a comment explaining it in the
  kernel config file is probably entirely adequate for the handful of
  people who still have such hardware.
 
  It is probably almost all 486 systems, and no pentium ones.
  They probably need a 'small' kernel anyway.
 
 I had a 486DX4 motherboard, now broken, that had PCI.  A good point
 about GENERIC not being ok.
 
  More interesting might be the embedded 486-like systems
  from soekris (etc). Not sure if any of those have graphics
  but they will normally run a generic kernel.
 
 The Soekris net5501 and net6501 don't have any VGA, and I think the rest
 also lack it.  But I see the point about the other SOC stuff.
 
 So just NO_PCI is descriptive enough; we should rename the file is
 going to stay.

Interesting thought is how many other drivers might be removable
from GENERIC because you wouldn't use the isa/eisa cards in a
pci system.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2015-03-11 Thread David Laight
On Wed, Mar 11, 2015 at 11:20:03AM -0400, Greg Troxel wrote:
 
 Alan Barrett a...@cequrux.com writes:
 
  On Sat, 07 Mar 2015, Martin Husemann wrote:
  Anything that has no PCI.
 
  Could we rename LEGACY to something more descriptive?  The problem
  with the name LEGACY is that it leaves people wondering whether or
  not particular hardware is old enough to be classified as legacy.
  If the test is Does it have an ISA bus without a PCI bus? then I'd
  suggest a name like ISA_NO_PCI.
 
 I agree about ISA_NO_PCI.   In addition, legacy is a really
 problematic word in technical English, because it usually means how
 people do things now, compared to the new thing I am proposing that I
 think everyone should do instead.

yes - you don't know how old 'legacy' is.

 I'm also not sure we need a new kernel config.  It seems the systems of
 interest are limited to 486 machines without PCI (and thus EISA
 probably), and that's a pretty narrow window around 1993-1994.  So
 leaving the lines commented out with a comment explaining it in the
 kernel config file is probably entirely adequate for the handful of
 people who still have such hardware.

It is probably almost all 486 systems, and no pentium ones.
They probably need a 'small' kernel anyway.

More interesting might be the embedded 486-like systems
from soekris (etc). Not sure if any of those have graphics
but they will normally run a generic kernel.


David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: [netbsd-7] xsrc/external/mit/MesaLib/dist/src/mapi

2015-02-17 Thread David Laight
On Sat, Feb 14, 2015 at 08:14:01AM +, Soren Jacobsen wrote:
 Module Name:  xsrc
 Committed By: snj
 Date: Sat Feb 14 08:14:01 UTC 2015
 
 Modified Files:
   xsrc/external/mit/MesaLib/dist/src/mapi [netbsd-7]: entry.c
 
 Log Message:
 Pull up following revision(s) (requested by mrg in ticket #514):
   external/mit/MesaLib/dist/src/mapi/entry.c: revision 1.2
 disable the use of 32 bit x86 asm code here, it (like the 64 bit code),
 does not work on netbsd and probably needs an x86 guru to fix.

Wouldn't it be more sensible to undefine USE_X86_ASM in the maefile?

David

-- 
David Laight: da...@l8s.co.uk


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

2015-01-24 Thread David Laight
On Wed, Jan 21, 2015 at 10:09:55PM +0900, Tetsuya Isaki wrote:
 At Wed, 15 Oct 2014 19:48:53 +0100,
 David Laight wrote:
   
   Log Message:
   In the exp2_values test case, provide separate expected return values
   for the float case, reflecting the actual exp2f() argument value after
   rounding to float precision.  Fixes PR lib/49256.  Thanks to Makoto
   Kamada and Tetsuya Isaki for the analysis.
  
  The reason I left the tests failing is that the results should be more
  accurate than the values that are actually returned.
  
  Changing the 'expected' values so that the tests pass is just wrong.
...
  I think the values ought to be accurate to one or two counts on the lsb
  of the mantissa.
 
 It is not an acuuracy problem of result, is an implicit type cast
 problem of argument.
 
 exp2()'s argument is double and exp2f()'s is float.
 
 As you know,
 exp2(7.7) means exp2((double)7.7) and
 exp2f(7.7) means exp2f((float)7.7).
 
 (double)7.7 = 0x1.ecccdp+2 in %a format
 (float)7.7  = 0x1.ecp+2 in %a format

I'd forgetten to allow for that difference, and the change didn't
mention it either.
I'd expect (double)exp2f(f) to differ from exp2((double)f) by only
1 or 2 counts in the mantissa. You definitely want to use the
lattet calculation to get the check value.

It is 'interesting' that (float)7.7 gets truncated rather than
rounded. I'd expect 7.7f to be 0x1edp+2, which would bring
the result into the old bounds.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2015-01-08 Thread David Laight
On Fri, Jan 02, 2015 at 06:11:48PM +0200, Alan Barrett wrote:
 On Fri, 02 Jan 2015, Christos Zoulas wrote:
 Log Message:
 Implement DIOCGMEDIASIZE and DIOCGSECTORSIZE from FreeBSD.
 
 This needs compat32 handling, at least for the u_int arg to
 DIOCGSECTORSIZE.
 
 Why not make it a fixed size, like uint32_t, so compat32 handling will
 not be needed?
 
 I think it was made u_int to match previous art by FreeBSD. Can you please
 explain why it needs compat32 handling?
 
 Sorry, it doesn't need compat32 handling, because all existing
 NetBSD platforms have 32-bit int, and all exiting NetBSD platforms
 have 64-bit off_t.

Specifying uint32_t and ensuring that 64bit fields are 'aligned'
is good practice and does no harm.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/sparc64/include

2014-12-31 Thread David Laight
On Tue, Dec 30, 2014 at 04:34:42PM -0600, Dennis Ferguson wrote:
 
 On 30 Dec, 2014, at 12:52 , David Laight da...@l8s.co.uk wrote:
  Is that the correct fix?
  Unless the rdpr actually accesses memory (don't think it does) then
  then problem is probably a missing 'volatile' instead.
  
  Certainly the way those asm functions are defined looks to be
  rather more obfuscated than necessary.
 
 Or maybe just get rid of the __constfunc?  I think that, plus
 the (void) argument list to the inline, is explicitly telling the
 compiler it should feel free to move the call absolutely anywhere
 it feels like placing it.

The relevant optimisations happen after the inline, so the function
definition shouldn't affect things.
I'm guessing that __constfunc just means that the compiler can merge
multiple calls (and cache the result), not at all sure that is
valid with a memory clobber.

 I'm a bit surprised the memory clobber by itself changed anything
 at all since I thought a memory clobber without a 'volatile' is
 supposed to refer only to memory-based input and output arguments
 to the asm(), of which there are none.

The compiler doesn't know which args to the asm reference memory
unless you tell it with an appropriate 'clobber' - either the
global one, or a ranged one based on one if the arguments.

In this case I suspect removing __constfunc and making the
asm volatile will force correct sequencing.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: xsrc/external/mit/MesaLib/dist/src/mesa/main

2014-12-31 Thread David Laight
On Tue, Dec 30, 2014 at 05:58:06PM -0500, Christos Zoulas wrote:
 Modified Files:
   xsrc/external/mit/MesaLib/dist/src/mesa/main: format_utils.c
 
 Log Message:
 The macros in this file generate a gigantic function that takes a long time
 to compile and a ton of memory. Split it per datatype so that each is 1/8th
 the size. On my 48GB amd64 box this results in 3x speedup.

What happens at run time ?
Sounds like somethink that is being inlined so much that it
will displace everything from the cache and run slowly because
it is always waiting for instruction fetches.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/sparc64/include

2014-12-31 Thread David Laight
On Wed, Dec 31, 2014 at 11:15:24AM +0100, Martin Husemann wrote:
 On Wed, Dec 31, 2014 at 10:05:22AM +, David Laight wrote:
  In this case I suspect removing __constfunc and making the
  asm volatile will force correct sequencing.
 
 Check the commit history.
 Can we make only the hypverisor call __constfunc?

Looks like the two changes are fighting in different directions.
One is trying to get the compiler to cache the result of the
function by calling it once at the top, the other to ensure
it only called when some other condition is true.
You can't have it both ways.

Maybe that one function shoukd be caching the result of
the CPU_IS_* macro.
Or maybe save the relevant register value in memory during
initialisation.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/atari/stand/installboot

2014-12-02 Thread David Laight
On Fri, Nov 14, 2014 at 02:43:39AM +0900, Izumi Tsutsui wrote:
 christos@ wrote:
 
  Module Name:src
  Committed By:   christos
  Date:   Thu Nov 13 17:19:29 UTC 2014
  
  Modified Files:
  src/sys/arch/atari/stand/installboot: installboot.c
  
  Log Message:
  fix strict aliasing violations
 
  -   *((u_int16_t *)bb-bb_xxboot + 255) = 0;
  -   *((u_int16_t *)bb-bb_xxboot + 255) = 0x1234 - abcksum(bb-bb_xxboot);
  +   sum = 0;
  +   memcpy(bb-bb_xxboot + 255, sum, sizeof(sum));
  +   sum = 0x1234 - abcksum(bb-bb_xxboot);
  +   memcpy(bb-bb_xxboot + 255, sum, sizeof(sum));
 
 I doubt your bb-bb_xxboot + 255 is the same place
 as the original (u_int16_t *)bb-bb_xxboot + 255
 (the cksum word looks at the end of 512 byte sector).
 
 memcpy(9) looks also awful for readers because it hides endianness..

(Having read all the thread...)

I'm pretty sure you can cast between a pointer to a union and
a pointer to one of its mmembers.

So under the assumption that the compiler will let you alias between
members of a union (if it doesn't then far more stuff will break)
the following is safe:

Define a union that contains a 'bb' and a u_int16_t [].
Declare a variable that is a pointer to the union.
Assign (with cast) the address of 'bb' to the pointer variable.
Access the other member of the union.

The only other alternative is to do all the accesses as 'char'.

I'd guess that if the compiler inlines memcpy() it can 'know' about the
types of the variables involved - and apply the 'strict aliasing'
rules in the same way that is applied the 'alignment' ones.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/atari/stand/installboot

2014-12-02 Thread David Laight
On Mon, Nov 24, 2014 at 02:36:51PM +, Taylor R Campbell wrote:
 
 In the case of abcksum, if you have
 
 uint8_t p[512];
 ...
 *((uint16_t *)p + 255) = 0;
 *((uint16_t *)p + 255) = abcksum(p);
 
 GCC may choose to evaluate abcksum before the zero assignment, because
 no uint16_t is allowed to alias a uint8_t, so the assignment of an
 lvalue with type uint16_t can't change the value of abcksum(p).

Not sure, the accesses of p[] inside abcksum() are allowed to alias
any other memory - this includes wherever was written to by the
*((uint16_t *)xxx) = 0; regardless of any expected knowledge of
the value of xxx.

For gcc you can add asm volatile(:::memory); to force it to
'forget' anything it thinks it knows about the contents of memory.

The original checksum code can be (mostly) fixed by only having
a single (uint16_t) cast.
You are then only left with problems caused if the checksum is inlined
and any earlier writes to the sector itself are still 'pending'.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-10-15 Thread David Laight
On Tue, Oct 07, 2014 at 12:40:04AM +0700, Robert Elz wrote:
 Date:Mon, 6 Oct 2014 14:26:44 +
 From:Havard Eidnes h...@netbsd.org
 Message-ID:  20141006142644.7693...@cvs.netbsd.org
 
   | Make this build again without debugging enabled; DPRINTF() can end up
   | as empty, and in an if conditional, you then need braces if that's the
   | only potential body.
 
 That change makes no sense to me - the original code was
 
   if (error)
   DPRINTF(());
 
 and even if DPRINTF() could produce nothing, the result would be
 
   if (error)
   ;
 
 which is perfectly good C (useless, but acceptable).  Adding { } around
 the ';' changes nothing.

Consider what happens if you write:
if (error)
DPRINTF((...));
else
fubar();

When DPRINTF() expands 'if (xxx) yyy' it all goes horribly wrong.

Do we need to support any compilers that don't support __VA_ARGS__ ?
Even microsoft's compiler almost supports it.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-10-15 Thread David Laight
On Tue, Oct 07, 2014 at 04:53:44PM +, Andreas Gustafsson wrote:
 Module Name:  src
 Committed By: gson
 Date: Tue Oct  7 16:53:44 UTC 2014
 
 Modified Files:
   src/tests/lib/libm: t_exp.c
 
 Log Message:
 In the exp2_values test case, provide separate expected return values
 for the float case, reflecting the actual exp2f() argument value after
 rounding to float precision.  Fixes PR lib/49256.  Thanks to Makoto
 Kamada and Tetsuya Isaki for the analysis.

The reason I left the tests failing is that the results should be more
accurate than the values that are actually returned.

Changing the 'expected' values so that the tests pass is just wrong.

What I should have got araound to doing is using the x87 instruction
to generate accurate 64bit mantissa (long double) results for the
values, and then looked at the sizes of the error terms.

I think the values ought to be accurate to one or two counts on the lsb
of the mantissa.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-10-15 Thread David Laight
On Tue, Oct 14, 2014 at 03:16:56AM +, John Nemeth wrote:
 Module Name:  src
 Committed By: jnemeth
 Date: Tue Oct 14 03:16:56 UTC 2014
 
 Modified Files:
   src/sys/arch/x86/x86: identcpu.c
 
 Log Message:
 Force x86_xsave_features to 0 when running under XEN for AMD
 processors.  This prevents the use of xsave and xrstor thus fixing
 the problem in PR/49150.  The basic problem is that the way AMD
 implements those instructions means that information can leak
 between domains so XEN treats them as privileged.
 
 XXX If anybody else comes up with a better / more proper fix, go
 for it.  However, this solves the problem I was having.  And, given
 that XEN being broken is pretty much a show-stopper for a release,
 something needed to be done.

One option would be to detect the fault at runtime.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/common/lib/libc/arch/i386/string/small

2014-09-26 Thread David Laight
On Mon, Sep 22, 2014 at 08:31:56PM +, Pierre Pronchery wrote:
 Module Name:  src
 Committed By: khorben
 Date: Mon Sep 22 20:31:56 UTC 2014
 
 Modified Files:
   src/common/lib/libc/arch/i386/string/small: strchr.S
 
 Log Message:
 Look for the character to locate before checking for the NUL character
 
 As documented in PR port-i386/49208, this fixes strchr(s, '\0'), as used by
 the FAT first-stage bootloader on x86 (bootxx_msdos).
 strchr(s, '\0') is otherwise equivalent to strlen(string), which would
 probably look nicer in the original file, dosfs.c from libsa.

FWIW 'strchr(s, 0)' is actually 's + strlen(s)'

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2014-08-15 Thread David Laight
On Wed, Aug 13, 2014 at 12:03:39PM +0900, Izumi Tsutsui wrote:
 david@ wrote:
 
   Note sysinst(8) requires some spare space in /tmp during installation,
   especially on using non-default settings on choosing installation sets 
   etc.
   on ports which don't use tmpfs or mfs during installation.
   (100KB would be enough for this newsmips case though)
  
  It is also useful if there is enough space for sysinst to drop core.
 
 I don't think it's so worth in the real world.

I added enough space to i386 (years ago) because the 'file system full'
messages (from failing to drop core) looked like a different problem.

Even on a system with as little as 32MB memory, allocating an extra meg
to the install ramdisk is unlikely to stop the install working.
IIRC the ramdisk is compressed on the install media - so the empty
blocks take almost no space there.

So really there is no reason not to a reasonable amount of free space.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/vax/include

2014-08-15 Thread David Laight
On Thu, Aug 14, 2014 at 09:17:33AM +, Martin Husemann wrote:
 Module Name:  src
 Committed By: martin
 Date: Thu Aug 14 09:17:32 UTC 2014
 
 Modified Files:
   src/sys/arch/vax/include: int_fmtio.h
 
 Log Message:
 intptr_t and uintptr_t are not long any more.

Gah - that means that the underlying type differs between architectures.
There must be a gcc config switch somewhere.
I wonder what they are on other 32bit archs (they mut be 'long' on 64bit
ones).

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/vax/include

2014-08-15 Thread David Laight
On Fri, Aug 15, 2014 at 08:14:06AM +0100, David Laight wrote:
 On Thu, Aug 14, 2014 at 09:17:33AM +, Martin Husemann wrote:
  Module Name:src
  Committed By:   martin
  Date:   Thu Aug 14 09:17:32 UTC 2014
  
  Modified Files:
  src/sys/arch/vax/include: int_fmtio.h
  
  Log Message:
  intptr_t and uintptr_t are not long any more.
 
 Gah - that means that the underlying type differs between architectures.
 There must be a gcc config switch somewhere.
 I wonder what they are on other 32bit archs (they mut be 'long' on 64bit
 ones).

I should have read the next commit message 

David

-- 
David Laight: da...@l8s.co.uk


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

2014-08-12 Thread David Laight
On Sun, Jul 13, 2014 at 04:34:59PM +0200, Martin Husemann wrote:
 On Sun, Jul 13, 2014 at 04:31:58PM +0200, Martin Husemann wrote:
  Why does lsof define _KERNEL ?
 
 Let me rephrase: we provide the userland important stuff when either _KERNEL
 or MAKEFS is defined - maybe MAKEFS should be renamed and lsof could use the
 new define instead of _KERNEL.
 
 MSDOSFS_MOUNT_USERLAND_DEFS or something.

or _KMEM_USER ?

David

-- 
David Laight: da...@l8s.co.uk


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

2014-08-12 Thread David Laight
On Wed, Aug 06, 2014 at 09:11:47AM +, Martin Husemann wrote:
 Module Name:  src
 Committed By: martin
 Date: Wed Aug  6 09:11:46 UTC 2014
 
 Modified Files:
   src/usr.sbin/sysinst: main.c
 
 Log Message:
 Make sysinst use the catalog files installed in /usr/share/sysinst (if
 available), fall back to . if not, or use build-in english otherwise.

Is that really the best order?
Wouldn't it be more useful to look in . first?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2014-08-12 Thread David Laight
On Mon, Aug 11, 2014 at 05:25:29AM +0900, Izumi Tsutsui wrote:
 riz@ wrote:
 
  Module Name:src
  Committed By:   riz
  Date:   Sun Aug 10 20:04:30 UTC 2014
  
  Modified Files:
  src/distrib/newsmips/floppies/ramdisk: Makefile
  src/sys/arch/newsmips/conf: INSTALL
  
  Log Message:
  Bump the ramdisk size; the longer paths on the build cluster are
  likely enough to put this over the edge.
  
  
  To generate a diff of this commit:
  cvs rdiff -u -r1.31 -r1.32 src/distrib/newsmips/floppies/ramdisk/Makefile
 
  -IMAGESIZE= 2560k
  +IMAGESIZE= 2660k
 
 Note sysinst(8) requires some spare space in /tmp during installation,
 especially on using non-default settings on choosing installation sets etc.
 on ports which don't use tmpfs or mfs during installation.
 (100KB would be enough for this newsmips case though)

It is also useful if there is enough space for sysinst to drop core.
Unless the system has very limited memory (in which case it really won't
run much) then adding a meg or two to the ramdisk size probably doen't
matter.
IIRC it is compressed in the install media.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-06-16 Thread David Laight
On Fri, Jun 13, 2014 at 10:42:26AM +, Joerg Sonnenberger wrote:
 Module Name:  src
 Committed By: joerg
 Date: Fri Jun 13 10:42:26 UTC 2014
 
 Modified Files:
   src/sys/compat/netbsd32: netbsd32_sysctl.c
 
 Log Message:
 Rename stack gap arguments.

stack gap ? that was expunged years ago...

David

-- 
David Laight: da...@l8s.co.uk


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

2014-05-16 Thread David Laight
On Fri, May 16, 2014 at 03:54:44PM +, David Holland wrote:
 
   Indeed rebooting with an updated kernel will give active NFS clients
   problems, but I am not sure we should realy care nor how we could
   possibly avoid this one time issue. We have changed encoding of
   filehandles before (at least once).
 
 I don't think this is a problem, but maybe I'll put a note in UPDATING.

Never mind that problem.
Consider what happens if you reboot with a different CD in the drive!

I once fixed a filesystem to use different faked inode numbers every
time a filesystem was mounted.
Without that NFS clients would write to the wrong file in the wrong FS.

The 'impossible to get rid of' retries for hard mounts were something
up with which I had to put. (A preposition is something you should not
end a sentence with.)

David

-- 
David Laight: da...@l8s.co.uk


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

2014-04-24 Thread David Laight
On Thu, Apr 24, 2014 at 10:56:29AM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Apr 24 14:56:29 UTC 2014
 
 Modified Files:
   src/usr.bin/netstat: main.c route.c
 
 Log Message:
 The sysctl code does not support verbose route printing that prints the
 internal route statistics. Restore the old kmem route printing code that
 was not just used for post-mortem displays. Reported by kardel@, test by
 netstat -nrvf inet

That seems a large backwards step...

David

-- 
David Laight: da...@l8s.co.uk


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

2014-04-23 Thread David Laight
On Tue, Apr 15, 2014 at 03:41:55PM +0200, Joerg Sonnenberger wrote:
 
 I remember a discussion about this topic from the LLVM lists and the
 reasons for the standard language on this are extremely weak. IIRC the
 *only* justification was for some platforms with broken (trapping)
 prefetch instructions.

That can only possibly matter if memcpy() is implemented with something
(equivalent to) 'rep movsb' - otherwise the broken prefetch would happen
during normal code sequences.

I'd have thought a zero length memcpy() would also be valid with (the valid)
pointer to 'just beyond' and item. A prefetch there could also fault.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/x68k/stand/boot_ustar

2014-04-23 Thread David Laight
On Tue, Apr 15, 2014 at 12:43:42PM -0700, Dennis Ferguson wrote:
 
 I'm pretty sure NetBSD could never run on a 68000 since the 68000
 had no memory management unit.  The 68010 and 68020 didn't have memory
 management units either, but Sun did proprietary MMUs for both (that's
 sun2 and sun3, respectively) and I think other companies may have done
 MMUs for the 68020.  The 68030 was the first CPU in the series to come
 with an MMU built in.

ISTR that either (or both) of the 68000 or 68010 don't save enough
state to make all address faults recoverable.
This stops you running OS that do paging - so you could only run
OS that swap entire processes.

The '020 saves the mid-instruction state for some faults (as well as the
actual fault reason) so they can be recovered.
(Just don't continue on a cpu with a different mask level.)

David

-- 
David Laight: da...@l8s.co.uk


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

2014-04-02 Thread David Laight
On Wed, Apr 02, 2014 at 06:37:21AM -0400, Christos Zoulas wrote:
 On Apr 2,  2:10pm, m...@eterna.com.au (matthew green) wrote:
 | i like this.. i think.
 | 
 | i wonder if this will break my system that boot from a different
 | device to the raid root device.
 
 I think it will.
 
 | eg, ultra10 whose prom can't talk to my sata disk, and boots from
 | a cf/ide fob with just ofwboot and netbsd (and attaches as wd0),
 | and mountroots from the raidframe on wd1+wd2.
 
 Well, there are different options here:
 1. is there a way to pass the root from ofwboot to netbsd?

There should be - I'd have though that loading the kernel
from other than the root parition was needed for some non-raid
configurations - eg large disks.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/distrib/utils/embedded/conf

2014-04-01 Thread David Laight
On Mon, Mar 31, 2014 at 02:18:45PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Mon Mar 31 18:18:45 UTC 2014
 
 Modified Files:
   src/distrib/utils/embedded/conf: x86.conf
 
 Log Message:
 remove swap; these days x86 machines don't need it.

What about for dumps?

Although I'm not at all sure what this particular file is for...

From the diff it looks like it needs for TLC in order to get sane
partition offsets.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-04-01 Thread David Laight
On Tue, Apr 01, 2014 at 07:16:18AM +, David Laight wrote:
 Module Name:  src
 Committed By: dsl
 Date: Tue Apr  1 07:16:18 UTC 2014
 
 Modified Files:
   src/sys/arch/x86/x86: x86_machdep.c
 
 Log Message:
 Revert most of the machdep sysctls to 32bit

I think I remember why I set to 64bit now.
Stopped all the lines wrapping 80 columns.
And having looked at how/where the data was stored thought it
wouldn't matter.

It is also a real PITA that the entire internals of the kernel sysctl
code are exposed to userspace.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-03-25 Thread David Laight
On Tue, Mar 25, 2014 at 05:17:44PM +, Joerg Sonnenberger wrote:
 Module Name:  src
 Committed By: joerg
 Date: Tue Mar 25 17:17:44 UTC 2014
 
 Modified Files:
   src/usr.sbin/rtsold: rtsold.c
 
 Log Message:
 Don't cast to time_t just to implicitly cast to uint32_t next.

Gah - I hadn't spotted that xtos had added a cast as well.
There is another cast that ought to be pointless two lines later.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/games

2014-03-23 Thread David Laight
On Sun, Mar 23, 2014 at 12:17:40AM +, David A. Holland wrote:
 Module Name:  src
 Committed By: dholland
 Date: Sun Mar 23 00:17:40 UTC 2014
 
 Modified Files:
   src/games: Makefile.inc
 
 Log Message:
 Add note cautioning against bothering with WARNS=6 until gcc improves
 (or -Wconversion is removed from WARNS=6) as it produces loads of false
 positives. The most entertaining of these that I've seen this afternoon:
 
 games/hack/hack.apply.c:143:22: error: conversion to 'unsigned char:1' from 
 'int' may alter its value [-Werror=conversion]
flags.move = multi = 0;

I agree.

If the compiler was doing some simple checks on the domain of the
value the it might be sensible, but otherwise is generates a lot
of false positives fo very little gain.

I dislike adding casts between integer types just to appease compiler
(and lint) warnings. They make the code unreadable and can hide
much more serious errors.

I'll live with -Wsign-compare even though that is sometimes painful.
Even then the compiler can 'know' that the signed variable never
contains a negative value - so need not emit a warning.

For instance with:
for (int i = 0; i  sizeof foo; i++)
it can't matter that the comparison is unsigned because 'i' can be
assumed to be non-negative (even if 'sizeof foo' is greater than MAXINT).


David

-- 
David Laight: da...@l8s.co.uk


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

2014-03-18 Thread David Laight
On Tue, Mar 18, 2014 at 03:30:09PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Tue Mar 18 19:30:09 UTC 2014
 
 Modified Files:
   src/usr.sbin/rtsold: rtsold.c
 
 Log Message:
 use time_t for time

It isn't a time_t type time.
The value is actually an interval in microseconds and bounded to less than
a second.

The code is doing:
long x = arc4random() % (1 * 100);
That really shouldn't generate a compiler warning.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-03-18 Thread David Laight
On Tue, Mar 18, 2014 at 08:21:19PM +, Christos Zoulas wrote:
 In article 20140318201420.go20...@snowdrop.l8s.co.uk,
 David Laight  da...@l8s.co.uk wrote:
 On Tue, Mar 18, 2014 at 03:30:09PM -0400, Christos Zoulas wrote:
  Module Name:   src
  Committed By:  christos
  Date:  Tue Mar 18 19:30:09 UTC 2014
  
  Modified Files:
 src/usr.sbin/rtsold: rtsold.c
  
  Log Message:
  use time_t for time
 
 It isn't a time_t type time.
 
 It is assigned to a time_t. long might not be wide enough; the value is
 unsigned and it is assigned to a signed type.

Only if 'long' is smaller than 100.
i386 fails on the line:
long interval = arc4random() /* uint32_t */ % 100;
If you use 'unsigned long' then amd64 fails a line later doing:
xxx.tv_usec = interval / 100;
The compiler should be able to tell that neither of those lines
is going to cause any problems.

There is no reason to force 32bit systems to do unnecessary 64bit maths.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-03-12 Thread David Laight
On Thu, Mar 13, 2014 at 12:32:38AM +0900, Mindaugas Rasiukevicius wrote:
 Taylor R Campbell campbell+netbsd-source-change...@mumble.net wrote:
 Date: Wed, 12 Mar 2014 16:16:32 +0200
 From: Jukka Ruohonen jruoho...@iki.fi
  
 On Wed, Mar 12, 2014 at 09:39:23AM +, Juergen Hannken-Illjes wrote:
  Restructure layer_lock() to always lock before testing for dead node.
  Use ISSET() to test flags, add assertions.
  
 As I wrote in the manual page, I'd rather see ISSET(3) et. al.
  disappear, i.e. these obscure rather than clarify...
  
  I disagree.  Phrases like `(vp-v_iflag  (VI_XLOCK | VI_CLEAN)) == 0'
  make my head's parser stumble -- there are just enough complements to
  juggle that it overwhelms my brain registers for the fast path.  I'd
  rather read `!ISSET(vp-v_iflag, (VI_XLOCK | VI_CLEAN))'.
 
 I disagree.  For kernel developers, that kind of bitwise arithmetics and
 masking ought to be intuitive.  If there is more logic and it gets long,
 then separate it:
 
 const bool foobar = (mask  (FOO | BAR)) == 0;
 const bool baz = (mask  BAZ) != 0;
 
 if (foobar  baz) ...

Except you really don't want the compiler to convert the value to
a 'bool'.

 ISSET() is somewhat okay (although I do not use it), but I particularly
 dislike __BIT() as I forget whether the 1st bit is n = 0 or whether this
 API tries to be fancy and it is n = 1.  1U  n is just straigtforward.

Or number from the other end...

Indeed, I have to go away and find the definitions and then realise
that they are just longhand!

I don't normally compare bit masking against zero, just:
if (var  BIT)
or
if (!(var  BIT))
to me they read better that way.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/public-domain/sqlite/lib

2014-03-07 Thread David Laight
On Fri, Mar 07, 2014 at 07:28:09PM +, Christos Zoulas wrote:
 In article 12922.1394219...@splode.eterna.com.au,
 matthew green  m...@eterna.com.au wrote:
 
 Christos Zoulas writes:
  Module Name:   src
  Committed By:  christos
  Date:  Fri Mar  7 18:37:48 UTC 2014
  
  Modified Files:
 src/external/public-domain/sqlite/lib: Makefile
  
  Log Message:
  change CC to HOST_CC to avoid trying to find stdarg.h in an unpopulated
  DSTDIR.
 
 won't this find the version of sqlite3.h on the host?
 it seems saner to elide this rule in the case that say
 !exists(${DESTDIR}/usr/include/stdarg.h) or something else
 ugly, until you figure out why it is being executed in the
 build too early.
 
 No, because there is a -I pointing to the directory that contains it.

Think I'd do
sqlite3.pc: ${SRCDIR}/sqlite3.h sqlite3.pc.in
@(V=$$( (echo '#include sqlite3.h'; echo @@@ SQLITE_VERSION @@@) | \
   ${CC} -E -I${SRCDIR} - | tail -1 | tr -d '')  \
   ${HOST_CC} -E -I${SRCDIR} - | grep '@@@' | tr -d ' @')  \
   ${TOOL_SED} -e s/@VERSION@/$$V/  ${.CURDIR}/sqlite3.pc.in \
${.TARGET})

That would be safer.
Although you might want to remove the target file on failure.
Or maybe change the last line to:
${.TARGET}.tmp  mv ${.TARGET}.tmp ${.TARGET}

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2014-03-06 Thread David Laight
On Wed, Mar 05, 2014 at 06:04:02PM +0200, Andreas Gustafsson wrote:
 
 2. I also object to the change of kern_syctl.c 1.247.
 
 This change attempts to work around the problems caused by the changes
 to the variable types by making sysctl() return different types
 depending on the value of the *oldlenp argument.
 
 IMO, this is a bad idea.  The *oldlenp argument does *not* specify the
 size of the data type expected by the caller, but rather the size of a
 buffer.  The sysctl() API allows the caller to pass a buffer larger
 than the variable being read, and conversely, guarantees that passing
 a buffer that is too small results in ENOMEM.
 
 Both of these aspects of the API are now broken: reading a 4-byte
 CTLTYPE_INT variable now works for any buffer size = 4 *except* 8,

That wasn't the intent of the change.
The intent was that if the size was 8 then the code would return
a numeric value of size 8, otherwise the size would be chnaged to
4 and/or ENOMEM (stupid errno choice) returned.

 and attempting to read an 8-byte CTLTYPE_QUAD variable into a buffer
 of less than 8 bytes is now guaranteed to yield ENOMEM *except* if the
 buffer size happens to be 4.

A request to read a CTLTYPE_QUAD variable into a buffer that is shorter
than 8 bytes has always been a programming error.
The intent of the change was to relax that is the length happened to be 4.

 IMO, this behavior violates both the
 letter of the sysctl() man page and the principle of least astonishment.

I'm not sure about the latter.
I run 'sysctl -a' and find the name of the sysctl I'm interested in.
The result is a small number so I pass the address and size of a integer
variable and then print the result.
(Or the value is rather large and I think it might exceed 2^31 so I
use an int64.)
The 'principle of least astonishment' would mean that I get the value
that 'sysctl -a' printed.

On a BE system I have to be extremely careful with the return values
from sysctl() or I see garbage.

Note that code calling systctl() has to either know whether the value
it is expecting is a string, structure, or number, or use the API calls
that expose the kernel internals in order to find out.

 Also, the work-around is ineffective in the case of a caller that
 allocates the buffer dynamically using the size given by an initial
 sysctl() call with oldp = NULL.

Code that does that for a numeric value will be quite happy with
either a 32bit of 64bit result.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-03-06 Thread David Laight
On Thu, Mar 06, 2014 at 12:30:25PM +, NONAKA Kimihiro wrote:
 Module Name:  src
 Committed By: nonaka
 Date: Thu Mar  6 12:30:25 UTC 2014
 
 Modified Files:
   src/sys/arch/i386/i386: cpufunc.S
 
 Log Message:
 fix to pass collect memory address to xrstor.

Gah ... :-(

FWIW I managed to get gcc 4.8 to optimise some fp loops to use the ymm
registers - the xsave/xrstor code seemed to worn an amd64.
But I don't have an i386 install on a new enough system.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2014-03-01 Thread David Laight
On Sat, Mar 01, 2014 at 08:31:42AM +0200, Alan Barrett wrote:
 On Thu, 27 Feb 2014, David Laight wrote:
 Modified Files:
  src/sys/kern: kern_sysctl.c
 
 +case CTLTYPE_INT:
 +/* Allow for 64bit read of 32bit value */
 +if (*oldlenp == sizeof (uint64_t)) {
 +qval = *(int *)d;
 +d_out = qval;
 +sz =  sizeof (uint64_t);
 +}
 +break;
 +case CTLTYPE_QUAD:
 +/* Allow for 32bit read of 64bit value */
 +if (*oldlenp == sizeof (int)) {
 +qval = *(uint64_t *)d;
 +ival = qval  0x1 ? qval : 0x;
 +d_out = ival;
 +sz =  sizeof (int);
 +}
 +break;
 
 This will fail if int is not a 32-bit type, because it uses hardcoded
 masks instead of adapting to the actual size.

It goes wrong if 'int' is larger than 'uint64_t' as well!
I'll fix it for 64bit int (modulo any signed v unsigned warnings).

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/uvm

2014-02-28 Thread David Laight
On Fri, Feb 28, 2014 at 09:07:40AM +0200, Alan Barrett wrote:
 On Wed, 26 Feb 2014, Matt Thomas wrote:
 Modified Files:
  src/sys/uvm: uvm_meter.c uvm_param.h
 
 Log Message:
 Add vm.min_address and vm.max_address which return VM_MIN_ADDRESS and
 VM_MAXUSER_ADDRESS.
 
 Do these need to use the old style #define constants
 instead of the new style (since 2005) CTL_CREATE idiom?

I certainly thought that no new 'fixed number' sysctls were supposed to
be added.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2014-02-24 Thread David Laight
On Sun, Feb 23, 2014 at 02:48:19PM -0800, John Nemeth wrote:
 On Feb 23, 10:35pm, David Laight wrote:
 } 
 } Module Name:src
 } Committed By:   dsl
 } Date:   Sun Feb 23 22:35:28 UTC 2014
 } 
 } Modified Files:
 } src/sys/arch/i386/i386: freebsd_machdep.c ibcs2_machdep.c
 } svr4_machdep.c
 } src/sys/arch/x86/include: fpu.h
 } src/sys/arch/x86/x86: fpu.c
 } 
 } Log Message:
 } Add fpu_set_default_cw() and use it in the emulations to set the default
 }   x87 control word.
 } This means that nothing outside fpu.c cares about the internals of the
 }   fpu save area.
 } New kernel modules won't load with the old kernel - but that won't matter.
 
  This requires a kernel version bump.

There'll be one soon enough, and was one a few days ago.
Mismatches in the modules will stop them loading.
In any case no one uses the three affected emulations - I'm not even sure
they really work at all!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/share/mk

2014-02-20 Thread David Laight
On Thu, Feb 20, 2014 at 01:30:27PM -0800, Matt Thomas wrote:
 
 On Jan 23, 2014, at 8:53 PM, Masao Uebayashi uebay...@gmail.com wrote:
 
  How about kernel modules?
 
 even kernel modules

Personally I think that DESTDIR should only contain the headers needed
for applications.
So some parts of sbin would only be buildable in a full source tree.
At the moment some headers seem to get released so that things
like libkvm (and other kernel grovellers) can be built.

If you look at the .d files for applcations you'll see that they end
up including all sorts of .h files that they really shouldn't need.

Separation in the kernel is even worse.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2014-02-13 Thread David Laight
On Thu, Feb 13, 2014 at 08:22:32AM +0100, Christoph Egger wrote:
   
  +ENTRY(fngetsw)
  +   fnstsw  %ax
  +   ret
  +
 
 Not sure if you mean fngetsw here (copy  paste) ?

No, I want to return the status word in %ax, rather that write
it to a memory location.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2014-02-05 Thread David Laight
On Wed, Feb 05, 2014 at 11:09:05PM +0100, Joerg Sonnenberger wrote:
 On Wed, Feb 05, 2014 at 06:52:22PM +, David Laight wrote:
  Module Name:src
  Committed By:   dsl
  Date:   Wed Feb  5 18:52:22 UTC 2014
  
  Modified Files:
  src/sys/arch/amd64/conf: Makefile.amd64
  src/sys/arch/i386/conf: Makefile.i386
  
  Log Message:
  Change the compiler options to explicitly specify:
-mno-mmx -mno-sse -mno-avx -mno-80387 -mno-fp-ret-in-387
  Since no-sse implies no-sse2 that should ensure that the compiler really
doesn't emit any instructions that might trap trying to use the FPU.
  On amd64 at least some of those are needed to stop the compiler
saving the registers to stack on every varargs function.
  It might be that -mno-sse did that before.
 
 This breaks clang. Please do not depend on one SSE option disabling
 another.

Without those flags I believe gcc might generate x87 instructions.
Quite possibly even for amd64.
See a very recent linux fix.
Try searching for: Disable generation of traditional x87 instructions

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2014-02-05 Thread David Laight
On Wed, Feb 05, 2014 at 03:09:59PM -0800, Matt Thomas wrote:
 
  
  This breaks clang. Please do not depend on one SSE option disabling
  another.
  
  Without those flags I believe gcc might generate x87 instructions.
  Quite possibly even for amd64.
  See a very recent linux fix.
  Try searching for: Disable generation of traditional x87 instructions
 
 I think Joerg is saying put in -mno-sse2 since that is not disabled
 by -mno-sse when using clang.

with the gcc I'm looking at (if I'm reading to code properly):
no-sse = no-sse2 = no-sse3 = no-ssse3  no-sse4a = no-sse4_1
= no-sse4_2 = no-avx = no-fma = no-fma4

You either need all of them, and the one that the next sub-version
of the cpu/compiler add, or you assume that the first ones imply the
latter - at least to some degree.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-02-02 Thread David Laight
On Sat, Feb 01, 2014 at 06:09:04PM -0800, John Nemeth wrote:
 } 
 } XEN might need the bits of fpuinit() that don't play with cr0.
 } In particular a fninit and setting TS.
 } Although I'm not 100% sure the TS is needed on a single cpu system.
 
  Xen domU is SMP capable.  NetBSD dom0 currently aren't, but
 hopefully it is in the works.

Probably best to make fpuinit() xen-save then.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-02-02 Thread David Laight
On Sun, Feb 02, 2014 at 01:27:45PM +0100, Manuel Bouyer wrote:
 On Sat, Feb 01, 2014 at 10:49:44PM +, David Laight wrote:
  On Sat, Feb 01, 2014 at 08:07:07PM +, Manuel Bouyer wrote:
   Module Name:  src
   Committed By: bouyer
   Date: Sat Feb  1 20:07:07 UTC 2014
   
   Modified Files:
 src/sys/arch/xen/xen: hypervisor.c
   
   Log Message:
   Revert previous: calling fpuinit() leads to a panic, as a domU is not
   allowed to manipulate cr0 directly. Xen doesn't need this, the fpu is
   handled by the hypervisor.
  
  I probably remember seeing that panic as well.
  
  XEN might need the bits of fpuinit() that don't play with cr0.
  In particular a fninit and setting TS.
  Although I'm not 100% sure the TS is needed on a single cpu system.
  It isn't there on amd64, but really it does no harm.
  Without it one process will have an indererminate fp state.
 
 It's been a while since I looked at this and I don't remmeber the details,
 but I don't think anything of fpuinit() is neeed for Xen. in netbsd-6
 npxattach() doens't do anything either for Xen (only set i386_fpu_present to
 1 and init npxdna_func, which seems to be done at compile time now).
 I think the FPU is initialised by the hypervisor. Or it's done by
 npxdna() on the first FPU use.
 
 With the code as is in HEAD now, all FPU-related atf tests pass on
 a Xen guest with 4 CPUs, so I assume it's working as expected.

Something needs to set the TS (task switched) flag when a new cpu
is added. Both amd64 and i386 'bare metal' have direct calls to
fpuinit() to do this.

If TS isn't set the first FP process that gets migrated to that cpu
won't fault on its first FP instruction. If it has just forked it
probably won't care, but otherwise it will all go horribly wrong.

It probably doesn't matter for the boot cpu - except that you (effectively)
clone 'random' FP registers instead of known 'initially zero' ones.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-02-02 Thread David Laight
On Sun, Feb 02, 2014 at 08:41:29PM +0100, Manuel Bouyer wrote:
 On Sun, Feb 02, 2014 at 06:53:55PM +, David Laight wrote:
  Something needs to set the TS (task switched) flag when a new cpu
  is added. Both amd64 and i386 'bare metal' have direct calls to
  fpuinit() to do this.
  
  If TS isn't set the first FP process that gets migrated to that cpu
  won't fault on its first FP instruction. If it has just forked it
  probably won't care, but otherwise it will all go horribly wrong.
 
 This is the clts/stts macros/functions, isn't it ?
 For Xen, this is done with the HYPERVISOR_fpu_taskswitch() hypercall.

Yes.

 For the boot CPU it's done in i386_proc0_tss_ldt_init().
 For other CPUs it will be done when a process is created/migrated
 to this CPU in i386_tls_switch(), because (l != ci-ci_fpcurlwp)
 will be true.

Yes, probably true.
I'd assumed that the call to fpuinit() served a purpose.

  
  It probably doesn't matter for the boot cpu - except that you (effectively)
  clone 'random' FP registers instead of known 'initially zero' ones.
 
 But these are zeroed on first call to the first npxdna() call,
 because the lwp won't have MDL_USEDFPU set at this time.

fork() ought to replicate the FP registers of the parent.
I think it saves the fpu state, and then copies it.
On bare metal it could base that descision of the state of the TS flag
(I don't think it does). But for XEN the equivalent isn't readable.

I'm about to commit code that inverts the default and assumes that the
fpu is present until fpuinit() detects it isn't.
That way it matters much less if it isn't called.
(No one runs 496SX)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/gpl3/gdb/dist/gdb

2014-02-02 Thread David Laight
On Sun, Feb 02, 2014 at 05:00:38PM -0500, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Sun Feb  2 22:00:38 UTC 2014
 
 Modified Files:
   src/external/gpl3/gdb/dist/gdb: inf-ptrace.c
 
 Log Message:
 Fix threading bug again.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.6 -r1.7 src/external/gpl3/gdb/dist/gdb/inf-ptrace.c
@@ -374,7 +380,7 @@
 XXX __NetBSD__: We used to pass this as the signal
 sig = ptid_get_lwp(ptid);
*/
-  ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal));
+  ptrace (request, pid, (PTRACE_TYPE_ARG3)1, sig);


You probably want to kill the comment from the last merge.

David

-- 
David Laight: da...@l8s.co.uk


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

2014-02-01 Thread David Laight
On Sat, Feb 01, 2014 at 08:07:07PM +, Manuel Bouyer wrote:
 Module Name:  src
 Committed By: bouyer
 Date: Sat Feb  1 20:07:07 UTC 2014
 
 Modified Files:
   src/sys/arch/xen/xen: hypervisor.c
 
 Log Message:
 Revert previous: calling fpuinit() leads to a panic, as a domU is not
 allowed to manipulate cr0 directly. Xen doesn't need this, the fpu is
 handled by the hypervisor.

I probably remember seeing that panic as well.

XEN might need the bits of fpuinit() that don't play with cr0.
In particular a fninit and setting TS.
Although I'm not 100% sure the TS is needed on a single cpu system.
It isn't there on amd64, but really it does no harm.
Without it one process will have an indererminate fp state.

I can't say I've actuallly looked a xen.
Presumably we somewhere compile the hypervisor as part of a kernel?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/opencrypto

2014-01-26 Thread David Laight
On Sun, Jan 26, 2014 at 11:04:31AM +1100, matthew green wrote:
 
   phase one:  disable auto-unload.
  
   Phase 1 has been done.
  
   for the whole kernel?
  
  No.  Only for this specific module.
 
 right - my suggestion was that since this problem affects a
 large class of modules, until that is fixed, we should
 disable auto unload globally.

Does almost everything get loaded and auto-unloaded at boot time?
If so that that isn't really a good idea.

It also sounds like manual unloads of drivers can happen when the
device is open - and that is also bad.
A manual unload probably isn't going to race with open or close though.

Disallowing unload completely would be a pain when developing drivers.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/bsd/file

2014-01-18 Thread David Laight
On Sat, Jan 18, 2014 at 11:53:09AM +0100, Martin Husemann wrote:
 On Sat, Jan 18, 2014 at 10:46:50AM +, Justin Cormack wrote:
  I believe gcc is correct here, you are only allowed to alias via a
  char pointer or the original type. Because you put a void pointer in
  instead gcc is right to complain.

You can also copy to 'void *' and then back to the original type.
In fact that is the only way you are allowed to use 'void *'.

 Right, but actually the complaint does not go away if we make the ? operator
 into an if () with two separate memcpy() calls without any casts.

I wouldn't expect it to.

There are two possible things gcc is complaining about:
1) It thinks you are accessing nh32 when only nh64 has been initialised
   (or v.v.).
2) It doesn't think the memcpy() calls actually writes the structures.

The first can be eliminated by unconditionally compiling both memcpy() calls.

If gcc doesn't think the memcpy() is writing to the structures then
it might completely mis-optimise the code.

For instance: if memcpy() is (modulo typing bugs):
#define memcpy(d, s, len) \
if (__builtin_constant(len)  (len) == 8) \
*(uint64_t *)(d) = *(uint64_t *)(s); \
else \
__memcpy(d, s, len) \

And you have:
struct { int32_t a; int32_t b } x, y = {1, 2};
then:
memcpy(x, y, 8);
printf(%u, x.a + x.b);
gcc can optimise away 'y' and the memcpy() completely, leaving 'x'
undefined.

While this might not affect the code in file, it might cause obscure
effects elsewhere.

The builtin memcpy() should have appropriate constraints to indicate
that it reads and writes the memory.
I think it should be possible the use asm statements that generate
comments but have m constraints (to a correctly sized char [])
top  tail in the memcpy() expansion so that the old buffer is read
before the copy and the new one afterwards.

Maybe:
#define reference_memory(ptr, len) \
asm volatile ( :: m ( *(struct {char x[len]; } *)(ptr)) )
#define memcpy(d, s, len) \
if (__builtin_constant(len)  (len) == 8) { \
reference_memory(s, 8); \
*(uint64_t *)(d) = *(uint64_t *)(s); \
reference_memory(d, 8); \
} else \
__memcpy(d, s, len) \

But my gnu asm is a bit weak!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/bsd/file

2014-01-17 Thread David Laight
On Fri, Jan 17, 2014 at 08:24:20PM +, Martin Husemann wrote:
 Module Name:  src
 Committed By: martin
 Date: Fri Jan 17 20:24:20 UTC 2014
 
 Modified Files:
   src/external/bsd/file: Makefile.inc
 
 Log Message:
 Make a gcc 4.8 warning non-fatal (couldn't find a way to avoid it, the
 data is initialized via memcpy to a void pointer, so the may be 
 uninitialized
 warning is not true)

If the problem actually caused by gcc failing to pair all the conditionals?
Compiling with 'clazz' a compile-time constant might show things.

Or, if memcpy() is defined in a header file and uses casts to optimise
inlined copies of fixed sizes it might be that the pointer-aliasing
rules mean that the actual structure might not have been written.

A possible solution to that is an asm statement with a memory
constraint for the buffer areas either side of the actual copy.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2014-01-15 Thread David Laight
On Wed, Jan 15, 2014 at 10:24:41PM +, Joerg Sonnenberger wrote:
 Module Name:  src
 Committed By: joerg
 Date: Wed Jan 15 22:24:41 UTC 2014
 
 Modified Files:
   src/sys/arch/amd64/acpi: acpi_wakecode.S
   src/sys/arch/amd64/amd64: mptramp.S
   src/sys/arch/i386/i386: mptramp.S
 
 Log Message:
 LLVM doesn't support data32/addr32, but is smart enough to figure the
 necessary prefixes out.

What justification does LLVM have for instering the prefixes
inside a .code16 section?
data32 addr32 lgdt(gdt_desc)
Nothing tells if the format of the descriptor - and it changes
between true 16bit mode and 32bit mode (which we want).

I'm not sure the addr32 is needed - since it is the operand size
that controls whether a 24 or 32 bit base is loaded.

I've certainly used lidt in real mode just to move the vector
table to a safe address (the 286 ice didn't like it).

David

-- 
David Laight: da...@l8s.co.uk


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

2013-12-26 Thread David Laight
On Thu, Dec 26, 2013 at 01:34:28PM -0500, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Dec 26 18:34:28 UTC 2013
...
 f:
 The types of the global variables 'timezone' and 'altzone' (if present)
 have been changed back to 'long'.  This is required for 'timezone'
 by POSIX, and for 'altzone' by common practice, e.g., Solaris 11.
 These variables were originally 'long' in the tz code, but were
 mistakenly changed to 'time_t' in 1987; nobody reported the
 incompatibility until now.  The difference matters on x32, where
 'long' is 32 bits and 'time_t' is 64.  (Thanks to Elliott Hughes.)

Doesn't that completely break binary compatibility for any applications
that access them as 64bit data items on 32bit systems?

On i386 (and other le systems) this can be fixed be ensuring there
are 4 zero bytes following the data.
In sparc (and other be systems) it requires renaming of the symbol itself.
I presume such a rename was done when time_t was extended to 64bits?

David

-- 
David Laight: da...@l8s.co.uk


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

2013-12-20 Thread David Laight
On Fri, Dec 20, 2013 at 01:17:54AM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Fri Dec 20 01:17:54 UTC 2013
 
 Modified Files:
   src/sys/dev/usb: if_udav.c
 
 Log Message:
 Add support for the brandless QF9700 USB 1.1 to 10Mbit/s Ethernet adapter.
 The multicast filter doesn't appear to work, so put the chip in promisc
 mode so that IPv6 NDP works.

Does it have a 'promiscuous for multicasts' option?
Possibly just setting all 1's in the filter will work.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libcurses

2013-11-10 Thread David Laight
On Sat, Nov 09, 2013 at 11:17:00AM +, Brett Lymn wrote:
 Module Name:  src
 Committed By: blymn
 Date: Sat Nov  9 11:16:59 UTC 2013
 
 Modified Files:
   src/lib/libcurses: add_wch.c addbytes.c addch.c addchnstr.c
   curses_private.h
 
 Log Message:
 Rename the old __waddbytes function to _cursesi_waddbytes and add a
 parameter that controls whether or not certain characters in the
 string are interpreted or not (things like tab being expanded).
...
 Fix the addchstr family functions so that they call _cursesi_waddbytes
 with character interpretation off as per SUSV2.

Does not expanding tabs actually make sense?
At a guess it would lead to tab characters being send to the terminal
and the display getting f**cked up.

I also suspect it will break applications that expect curses to
expand tabs.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-10-20 Thread David Laight
On Thu, Oct 17, 2013 at 07:56:17PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Oct 17 23:56:17 UTC 2013
 
 Modified Files:
   src/lib/libc/gen: arc4random.c
 
 Log Message:
 remove always inline because new gcc bitches.

In that function isn't inlined then the generated code will be
extremely horrid.

The 'always inline' was there for a reason.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sbin/gpt

2013-10-20 Thread David Laight
On Sat, Oct 19, 2013 at 01:19:03AM +, John Nemeth wrote:
 Module Name:  src
 Committed By: jnemeth
 Date: Sat Oct 19 01:19:03 UTC 2013
 
 Modified Files:
   src/sbin/gpt: gpt.8
 
 Log Message:
 type fix: accommodate. - accomodate.

Why? The correct spelling has two 'm's
At least in accommodation, can't imagine accommodate being different.

David

-- 
avid Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand/pxeboot

2013-10-20 Thread David Laight
On Sat, Oct 19, 2013 at 08:16:16PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Sun Oct 20 00:16:16 UTC 2013
 
 Modified Files:
   src/sys/arch/i386/stand/pxeboot: pxe_call.S
 
 Log Message:
 Move an instruction above .code16 so that it produces an R_386_32 instead
 of an R_386_16 relocation, which is truncated to fit. XXX: untested.

Broken - it would have to be moved to before the call to prot_to_real.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-10-20 Thread David Laight
On Sun, Oct 20, 2013 at 11:26:24AM +0200, Martin Husemann wrote:
 On Sun, Oct 20, 2013 at 10:05:30AM +0200, Alan Barrett wrote:
  For example:
  
  #define raise_ipi(cpi,lvl) do {\
  -  volatile int x; \
  +  int x;  \
 (cpi)-intreg_4m-pi_set = PINTR_SINTRLEV(lvl); \
  -  x = (cpi)-intreg_4m-pi_pend;  \
  +  x = (cpi)-intreg_4m-pi_pend; __USE(x);\
  } while (0)
 
 I don't think this change is incorrect, but I would do this differently.
 
 If the RHS is not properly marked volatile, the compiler might cache the
 value and repeatadly store the same value again, so the volatile hidden
 somewhere in the RHS is important. I wonder, however, why it does not
 properly warn here (loss of qualifiers), so we should realy investigate
 this closer.
 
 However, assuming the RHS is ok, it is IMHO better to get rid of i 
 completely
 and do:
 
 #define raise_ipi(cpi,lvl)do {\
   (cpi)-intreg_4m-pi_set = PINTR_SINTRLEV(lvl); \
   (void)(cpi)-intreg_4m-pi_pend;\
 } while (0)

If pi_pend isn't volatile you need something like:
   *(volatile * typeof ((cpi)-intreg_4m-pi_pend))(cpi)-intreg_4m-pi_pend

You might choose to know the type :-)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand/pxeboot

2013-10-20 Thread David Laight
On Sun, Oct 20, 2013 at 07:49:55PM +0100, David Laight wrote:
 On Sat, Oct 19, 2013 at 08:16:16PM -0400, Christos Zoulas wrote:
  Module Name:src
  Committed By:   christos
  Date:   Sun Oct 20 00:16:16 UTC 2013
  
  Modified Files:
  src/sys/arch/i386/stand/pxeboot: pxe_call.S
  
  Log Message:
  Move an instruction above .code16 so that it produces an R_386_32 instead
  of an R_386_16 relocation, which is truncated to fit. XXX: untested.
 
 Broken - it would have to be moved to before the call to prot_to_real.

Actually, if the value is more that 2^16 it is all broken anyway.

When I changed the disk /boot so that it could exceed 64k I didn't
change pxeboot (because I couldn't test it).
From the code in pxe_call.S it looks as though %cs, %ds, %es (and maybe %ss)
all have to be the same.
If you look at the disk (etc) bios calls you'll see that they set %ds/%es
correctly for the 32bit (well 20bit) linear address - which this code
doesn't do.

So the value has to fit in an R_386_16 relocation.

If it doesn't, it migth explain some of the panics in the pxe code.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/librumpuser

2013-09-26 Thread David Laight
On Thu, Sep 26, 2013 at 12:41:51AM +, Mindaugas Rasiukevicius wrote:
 Module Name:  src
 Committed By: rmind
 Date: Thu Sep 26 00:41:51 UTC 2013
 
 Modified Files:
   src/lib/librumpuser: rumpuser_pth.c
 
 Log Message:
 Give RUMP mutex and rwlock their own cache-line.  Also give a separate
 cache-line for the rwlock's reader counter.

You'd be much better off ordering the fields to avoid ping-pong
of data.

IIRC one of the new (big) ppc has 256 byte cache lines.
Cache line aligning data is then getting silly.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-09-14 Thread David Laight
On Sat, Sep 14, 2013 at 08:28:15PM +, Soren S. Jorvang wrote:
 Module Name:  src
 Committed By: soren
 Date: Sat Sep 14 20:28:15 UTC 2013
 
 Modified Files:
   src/sys/dev/usb: usbdevs
 
 Log Message:
 Add ASIX AX88179 USB 3.0 ethernet controllers.

Does NetBSD have a driver for them?
I presume it would have to be USB2 mode??

I've access to an adapter.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS import: src/external/bsd/less/dist

2013-09-05 Thread David Laight
On Wed, Sep 04, 2013 at 07:35:05PM +, Matthias Scheler wrote:
 Module Name:  src
 Committed By: tron
 Date: Wed Sep  4 19:35:05 UTC 2013
 
 Update of /cvsroot/src/external/bsd/less/dist
 In directory ivanova.netbsd.org:/tmp/cvs-serv10121
 
 Log Message:
 Import version 458 of less. Changes since version 444:
...
 * Don't quit if syntax errors are found in command line options.

Why is that an improvement?
If I mistype an argument something I wanted to happen isn't happening
and I need to be told.

I've been fighting 'netperf' which seems to accept absolutely any
crap on the command line, exceptionally difficult to determine what
it did.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libutil

2013-08-12 Thread David Laight
On Wed, Aug 07, 2013 at 10:51:59PM +, Paul Goyette wrote:
 Module Name:  src
 Committed By: pgoyette
 Date: Wed Aug  7 22:51:59 UTC 2013
 
 Modified Files:
   src/lib/libutil: snprintb.3
 
 Log Message:
 Add an example using snprintb_m()
 
 Replace \*[Gt] and \*[Lt] with the simple characters  and  (OK wiz)
 
 XXX Note that the examples currently do not compile with GCC!  The hex
 XXX character sequences such as \x10CACHE are being parsed as longer
 XXX than 2-hex-digit strings!

I would change all the examples to use concatenated strings.
As well as avoiding the above problem it makes them much, much, much
easier to read.

I remember changing some of the source files to split the strings.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: [riastradh-drm2] src/sys/external/bsd/drm2/include/linux

2013-07-24 Thread David Laight
On Wed, Jul 24, 2013 at 02:12:29AM +, Taylor R Campbell wrote:
 Module Name:  src
 Committed By: riastradh
 Date: Wed Jul 24 02:12:29 UTC 2013
 
 Modified Files:
   src/sys/external/bsd/drm2/include/linux [riastradh-drm2]: types.h
 
 Log Message:
 Add __le16, __le32, and __le64 to linux/types.h.
 
 As far as I'm aware, these aliases are correct, and the type doesn't
 actually have any effect on the byte order of reads and writes.

Correct - they are for the static analysis tools.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: [riastradh-drm2] src/sys/external/bsd/drm2/dist/drm/i915

2013-07-24 Thread David Laight
On Wed, Jul 24, 2013 at 03:47:07AM +, Taylor R Campbell wrote:
 Module Name:  src
 Committed By: riastradh
 Date: Wed Jul 24 03:47:07 UTC 2013
 
 Modified Files:
   src/sys/external/bsd/drm2/dist/drm/i915 [riastradh-drm2]: intel_panel.c
 
 Log Message:
 Kludge around max as a local variable in intel_panel.c.

Possibly adding a #define for max/min before/after including the
netbsd header that defines these as a function/define would solve
this in the wrapper headers?

David

-- 
David Laight: da...@l8s.co.uk


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

2013-07-23 Thread David Laight
On Sun, Jul 21, 2013 at 06:35:47PM +0200, Marc Balmer wrote:
 Am 20.07.13 18:44, schrieb Roy Marples:
  David Holland dholland-sourcechan...@netbsd.org writes:
  On Fri, Jul 19, 2013 at 07:15:52PM -0700, Erik Fair wrote:
 * dhcpcd will now assign a short hostname by default
  To use a FQDN hostname, set this in dhcpcd.conf(5)
  env hostname_fqdn=YES
   
This is the wrong default, too - hostname should always be FQDN.
 
  This is far from universally agreed upon.
 
  However, ISTM that if dhcpcd is going to set the hostname at all
  (which is usually wrong) it should set the hostname the dhcp server
  provides and not try to munge it.
  
  Which hostname should dhcpcd set?
 
 imo, dhcpcd should not set a hostname at all, at least not by default.

Maybe it should only set it if no other hostname has been set?

I'm not sure I've ever seen a system where the 'hostname' is a FQDN.
Historically (ancient) it wouldn't be - because FQDN didn't exist!
Neither Solaris nor any SVR4 (or unixware) used FQDN.
A SUSE9 system I have at work (PII-450 and still used as an xterm)
gets it IP from DNS and hostname from a DHCP reverse lookup and
only uses the shortname.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-07-23 Thread David Laight
On Mon, Jul 22, 2013 at 02:52:02PM +, Soren S. Jorvang wrote:
 Module Name:  src
 Committed By: soren
 Date: Mon Jul 22 14:52:02 UTC 2013
 
 Modified Files:
   src/sys/dev/pci: puc.c
 
 Log Message:
 Oops.

Not the best of commit messages!
'Fix compilation' would be better.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-07-01 Thread David Laight
On Wed, Jun 26, 2013 at 08:37:35PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Jun 27 00:37:35 UTC 2013
 
 Modified Files:
   src/sys/arch/x86/x86: tsc.c
 
 Log Message:
 detect a bad msr tsc and don't use it.

+   tsc_good = (cpu_feature[0]  CPUID_MSR) != 0  rdmsr(MSR_TSC) != 0;

There is a very small chance that the TSC will read as zero.
So the check should probably be:
tsc_good = (cpu_feature[0]  CPUID_MSR) != 0 
(rdmsr(MSR_TSC) != 0 || rdmsr(MSR_TSC) != 0);

The:
+extern int cpu_msr_tsc;
looks bogus - debug ??

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sbin/disklabel

2013-05-15 Thread David Laight
On Wed, May 15, 2013 at 09:48:03AM +0900, tsugutomo.en...@jp.sony.com wrote:
 Christos Zoulas chris...@astron.com writes:
 
  Are you worried about efficiency here? Yes, you can fix it differently 
  by checking if LABEL_OFFSET fits.
 
 I don't worry about efficiency.
 
 I've just noticed build failure on i386, and wondering what was wrong.
 
 I thought it would be better if warned when LABEL_OFFSET exceeds
 bootarea_len, but strictly speaking it is different matter.

I remember there being 2 main constraints:
1) Everything has to fit in the first 8k of the disk.
2) The netbsd label must not cross a 512 byte boundary.

The second constraint stopped a lot of architectures have a the default
number of partitins increased.

One problem is that non-boot media and exchangeable media need to be
read by different sustems that have different LABEL_SECTOR and
LABEL_OFFSET. I don't remember there being a was of deterimining
how much space was allocated on a volume for the label.
The 'partition count' is used to mean 'the number of fields with
values in them', possibly the checksum could be used (assuming that
any zeros can be over written).

Then there is the issue of 'c' and 'd'.
The x86 kernel sets the in memory 'c' and 'd' based on the disk size (etc)
regardless of the disklabel contents.
The 'c' partition info is almost never used (I think it is/was used to
determine the label secton that can't be written to).
If you put a disk where 'c' is raw, and 'd' is u user partition into an x86
system the 'd' partition could be made available as wd0c!


David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: [matt-nb5-mips64] src/sys

2013-05-14 Thread David Laight
I think this broke core dumps on mips ...

On Sun, Aug 23, 2009 at 03:38:19AM +, Matt Thomas wrote:
 Module Name:  src
 Committed By: matt
 Date: Sun Aug 23 03:38:19 UTC 2009
 
 Modified Files:
   src/sys/arch/mips/include [matt-nb5-mips64]: types.h
   src/sys/arch/mips/mips [matt-nb5-mips64]: mips_machdep.c
   process_machdep.c
   src/sys/kern [matt-nb5-mips64]: core_elf32.c sys_process.c
   src/sys/sys [matt-nb5-mips64]: ptrace.h
 
 Log Message:
 Change lazy fp load/save is done.  fpcurlwp is never NULL.
 If no current lwp has the FP, then fpcurlwp is set to lwp0.
 this allows many check for NULL and avoids a few null-derefs.
 Since savefpregs clear COP1, loadfpregs can be called to reload
 fpregs.  If it notices that situation, it just sets COP1 and returns
 Save does not reset fpcurlwp, just clears COP1.  load does set fpcurlwp.
 
 If MIPS3_SR_FR is set, all 32 64-bit FP registers are saved/restored via Xdc1.
 If MIPS3_SR_FR is clear, only 32 32-bit FP register are saved/restore via 
 Xwc1.
 This preserves the existing ABI.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.43.36.2 -r1.43.36.3 src/sys/arch/mips/include/types.h
 cvs rdiff -u -r1.205.4.1.2.1.2.2 -r1.205.4.1.2.1.2.3 \
 src/sys/arch/mips/mips/mips_machdep.c
 cvs rdiff -u -r1.29.62.1 -r1.29.62.2 src/sys/arch/mips/mips/process_machdep.c
 cvs rdiff -u -r1.32.16.1 -r1.32.16.2 src/sys/kern/core_elf32.c
 cvs rdiff -u -r1.143.4.1 -r1.143.4.1.4.1 src/sys/kern/sys_process.c
 cvs rdiff -u -r1.40 -r1.40.28.1 src/sys/sys/ptrace.h
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.
 

The change to core_elf32.c is mostly:

@@ -452,14 +456,19 @@ ELFNAMEEND(coredump_note)(struct proc *p
 #ifdef PT_GETFPREGS
notesize = sizeof(nhdr) + elfround(namesize) + elfround(sizeof(freg));
if (iocookie) {
+   size_t freglen = sizeof(freg);
uvm_lwp_hold(l);
+#ifdef __HAVE_PROCESS_XFPREGS
+   error = elf_process_read_xfpregs(l, freg, freglen);
+#else
error = elf_process_read_fpregs(l, freg);
+#endif
uvm_lwp_rele(l);
if (error)
return (error);
 
nhdr.n_namesz = namesize;
-   nhdr.n_descsz = sizeof(freg);
+   nhdr.n_descsz = freglen;
nhdr.n_type = PT_GETFPREGS;

error = ELFNAMEEND(coredump_writenote)(p, iocookie, nhdr,


However this code is called twice, once with iocookie == NULL in order
to find out how big everything will be, then again with iocookie != NULL
to actually do the writes.

So changing the size of the register area on the second pass is going to
lead to corrupt core files.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/share/mk

2013-05-03 Thread David Laight
On Fri, May 03, 2013 at 03:55:22PM +, Matt Thomas wrote:
 Module Name:  src
 Committed By: matt
 Date: Fri May  3 15:55:22 UTC 2013
 
 Modified Files:
   src/share/mk: bsd.own.mk
 
 Log Message:
 Use !empty(MACHINE_ARCH:Mearm*) instead of ${MACHINE_ARCH:Mearm*} != 

Any reason?
Make finds it much harder to parse empty().

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2013-04-21 Thread David Laight
On Sat, Apr 20, 2013 at 02:17:17PM -0700, Matt Thomas wrote:
 
 On Apr 20, 2013, at 11:04 AM, Christos Zoulas chris...@netbsd.org wrote:
 
  Module Name:src
  Committed By:   christos
  Date:   Sat Apr 20 18:04:41 UTC 2013
  
  Modified Files:
  src/sys/kern: kern_exec.c
  
  Log Message:
  don't attempt to load elf64 on 32 bit machines
 
 if the load address doesn't fit in 4GB.
 
 I've run elf64(n64) on mips n32 kernel with no problems.

Isn't an n32 kernel a 64bit one that uses 32bit pointers?
A little unusual.

Doesn't that require a COMPAT_64 layer in the kernel?
Since user pointers will be 64bit and kernel ones only 32.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-04-16 Thread David Laight
On Tue, Apr 16, 2013 at 07:34:58PM +, Joerg Sonnenberger wrote:
 Module Name:  src
 Committed By: joerg
 Date: Tue Apr 16 19:34:58 UTC 2013
 
 Modified Files:
   src/common/lib/libc/stdlib: _strtol.h _strtoul.h
 
 Log Message:
 Do not use isalpha here, since we explicitly only support the Portable
 Character Set as base and in theory a locale could define a ASCII
 control character as letter, resulting in negations. Also avoid isdigit
 here to give the compiler a better chance of deciding whether an
 unsigned compare or a jump table is a better option, both are very
 likely better choices than the memory indirection.

There really ought to be a way of requesting the straight ASCII
versions of the is functions even after a local has been set.
A lot of code will use isalpha() to check for valid variable names (etc)
and really doesn't want locale-specific alpha characters be valid.
(Otherwise scripts become non-portable.)

OTOH (unsigned)(ch - '0') = 9 is probably the fastest isdigit()
on any modern cpu.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: othersrc/usr.bin/dholland-make2

2013-03-23 Thread David Laight
On Sat, Mar 23, 2013 at 09:33:28PM +, David A. Holland wrote:
 Module Name:  othersrc
 Committed By: dholland
 Date: Sat Mar 23 21:33:28 UTC 2013
 
 Modified Files:
   othersrc/usr.bin/dholland-make2: make.c
 
 Log Message:
 Improve the toBeMade queue code so it doesn't do bulk array shifts
 every time you remove something from the queue head. If this turns out
 not to be good enough it can be tuned and/or made into a circular
 queue later.
 
 (Also, in the long run, the non-tail insertions may go away; they seem
 to chiefly be associated with the .ORDER implementation and may not be
 needed after the primary graph structure gets some strengthening.)

I was thinking of giving up on the inital attempt at a 'graph'.
Instead just remember all the depndencies for each node.

Once the makefile has been parsed (so all the dependencies are linked
up) scan all the nodes and add any with no unmade dependencies to a
list 'things that can be built'.

After building something, scan through the targets that depend on the
now-built item, if that is their last dependency then move on to the
list of buildable items.

That will look at the nodes far less often than the hack I had to do
to get .ORDER working.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib

2013-03-22 Thread David Laight
On Fri, Mar 22, 2013 at 12:08:31PM +0900, Joerg Sonnenberger wrote:
 On Thu, Mar 21, 2013 at 12:49:12PM -0400, Christos Zoulas wrote:
  Module Name:src
  Committed By:   christos
  Date:   Thu Mar 21 16:49:12 UTC 2013
  
  Modified Files:
  src/lib/libc/include: reentrant.h
  src/lib/libc/thread-stub: thread-stub.c
  src/lib/libpthread: Makefile pthread.c pthread_cancelstub.c
  pthread_cond.c pthread_int.h pthread_misc.c pthread_mutex.c
  pthread_once.c pthread_rwlock.c pthread_specific.c pthread_tsd.c
  
  Log Message:
  - Allow libpthread to be dlopened again, by providing libc stubs to 
  libpthread.
  - Fail if the dlopened libpthread does pthread_create(). From manu@
  - Discussed at length in the mailing lists; approved by core@
  - This was chosen as the least intrusive patch that will provide
the necessary functionality.
  XXX: pullup to 6
 
 This is wrong and unnecessary as pointed out on the mailing lists. Why
 are technical objections silently ignored now? Especially if they add
 overhead without any gain and make the error case more mysterious.

Does it even work?
On the face of it pthread__init() will be called by dlopen().
So __uselibcstub will get set to zero.

I also suspect thatthe pthread__errorfunc() calls rely on
pthread__diagnostic betin set - by pthread__init().

I think pthread_init() needs to be looking at some value that
libc set before calling main.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/netinet6

2013-03-19 Thread David Laight
On Mon, Mar 18, 2013 at 07:31:39PM +, Greg Troxel wrote:
 Module Name:  src
 Committed By: gdt
 Date: Mon Mar 18 19:31:39 UTC 2013
 
 Modified Files:
   src/sys/netinet6: ip6_output.c
 
 Log Message:
 Initialize variable used as (conditional) result parameter.
 
 ip6_insertfraghdr either sets a result parameter or returns an error.
 While the caller only uses the result parameter in the non-error case,
 knowing that requires cross-module static analysis, and that's not
 robust against distant code changes.  Therfore, set ip6f to NULL
 before the function call that maybe sets it, avoiding a spuruious
 warning and changing the future possible bug from an unitialized
 dereference to a NULL deferrence.

'If it returns fail it hasn't changed anything' is quite a common
property of functions. In fact I'd tend to expect it.

Cross module analysis isn't really a big issue, the actual problem
is when a compiler does a deeper analysis of a local function and
fails to spot the relationship.

There are some efficiency reasons for using multiple 'error' pointer
values so that the errno value can be returned in the same field.
They all tend to lead to coding errors!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/tests/fs/common

2013-03-19 Thread David Laight
On Tue, Mar 19, 2013 at 07:43:06PM +1100, matthew green wrote:
 
   The zfs tests are failing when run as an unprivileged user in
   thread_create() ... because default limit for threads (160) is too
   low ! When run as root, the limit value (2500) seems to be high enough.
  
  So should we increase the default limits?  It's always a tough call to
  set limits for a large ensemble of hardware, big and small, but it also
  seems that limits should be big enough to run our test cases.
 
 i think the default thread limit should be more on the order of 1024 
 to 4096.

Both the soft and hard limits for all the rlimits really need looking at.
In most cases the hard limits are much too high!

I'd have thought the soft thread limit should be relatively low (maybe
more than 160), and the hard limit say 1024 (dunno really).

Similarly for fds, maybe 256 and 4096.
At the moment there are nice local-user DoS because of the hard limit
values.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2013-03-12 Thread David Laight
On Tue, Mar 12, 2013 at 01:31:21PM +1100, matthew green wrote:
 
   Modified Files:
 src/sys/kern: subr_pool.c
   
   Log Message:
   In pool_cache_put_slow(), pool_get() can block (it does mutex_enter()),
   so we need to retry if curlwp took a context switch during the call.
  
  I didn't think mutex_enter() blocked - isn't it a spinlock.
  Which means that if things are going wrong they can go wrong
  even if the mutex is available immediately.
 
 netbsd kernel mutexes can be either, but this one is a spinlock yeah.

Hmmm I thought that internal test had gone with lockmgr :-)
My guess is that every call site knows whether sleeping is allowed,
so it could be a property of the mutex type.
Then there would be no confusion about whether the code might sleep.

I know the uncontended path is now common to both (and fast) - unlike
lockmgr which took a couple of 100 instructions to decide what to do.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2013-03-11 Thread David Laight
On Mon, Mar 11, 2013 at 09:37:54PM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Mon Mar 11 21:37:54 UTC 2013
 
 Modified Files:
   src/sys/kern: subr_pool.c
 
 Log Message:
 In pool_cache_put_slow(), pool_get() can block (it does mutex_enter()),
 so we need to retry if curlwp took a context switch during the call.

I didn't think mutex_enter() blocked - isn't it a spinlock.
Which means that if things are going wrong they can go wrong
even if the mutex is available immediately.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-03-09 Thread David Laight
On Sat, Mar 09, 2013 at 12:19:08AM +0100, Martin Husemann wrote:
 On Fri, Mar 08, 2013 at 11:03:23PM +, David Laight wrote:
  Should the 'no timeout' case (try to) check that the elapsed time is
  less than one tick?
 
 I gave the (partly virtual) test platforms a bit more slope and made it  1s
 instead of 1 tick, but otherwise: yes.

If the test did:
poll(0, 0, 1);
gettimeofday();
run_test();
gettimeofday();
It should be able to distinguish between the 'no sleep' and 'sleep
for one tick' cases.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-03-09 Thread David Laight
On Sat, Mar 09, 2013 at 10:10:45AM +, Michael van Elst wrote:
 Module Name:  src
 Committed By: mlelstv
 Date: Sat Mar  9 10:10:45 UTC 2013
 
 Modified Files:
   src/sys/arch/arm/arm: disksubr_acorn.c
 
 Log Message:
 Errors are supposed to be negative errno numbers, not -1 which is interpreted
 as EPERM.

Hmmm... NetBSD usually uses +ve errno values.

David

-- 
David Laight: da...@l8s.co.uk


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

2013-03-08 Thread David Laight
On Fri, Mar 08, 2013 at 05:01:54PM +, Martin Husemann wrote:
 Module Name:  src
 Committed By: martin
 Date: Fri Mar  8 17:01:54 UTC 2013
 
 Modified Files:
   src/tests/lib/libc/sys: Makefile
 Added Files:
   src/tests/lib/libc/sys: t_sigtimedwait.c
 Removed Files:
   src/tests/lib/libc/sys: t_sigtimedwait_pr_47625.c
 
 Log Message:
 Rename testprogram and make it more general by adding other testcases.
 One commented out, I didn't manage to get all signal handling correct
 for now.

Should the 'no timeout' case (try to) check that the elapsed time is
less than one tick?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/bsd/kyua-atf-compat/dist

2013-02-25 Thread David Laight
On Mon, Feb 25, 2013 at 06:54:13PM +, David Holland wrote:
 On Mon, Feb 25, 2013 at 06:49:51PM +, Julio Merino wrote:
   Log Message:
   Cherry-pick upstream change d0daf9983f5a0e635f1127dbc827aa114daa90d8:
   
   Fix broken variable parsing with NetBSD's /bin/sh
   
   Quote the expansion of a $() command that was not properly surrounded
   by quotes so that this runs properly with NetBSD's /bin/sh.
 
 In what way was it broken? (And where's the PR?)

I suspect that field splitting is applied to the RHS of assignments
when they are on local or export lines.

eg:
  (x=a b; b=fubar; export y=$x; echo $y; (echo $b))
outputs a and fubar (on two lines).

David

-- 
David Laight: da...@l8s.co.uk


  1   2   3   >