Re: CVS commit: src/external/bsd/dhcp/dist/common

2014-03-06 Thread NONAKA Kimihiro
Hi,

2014-03-07 10:04 GMT+09:00 Christos Zoulas :

> Module Name:src
> Committed By:   christos
> Date:   Fri Mar  7 01:04:30 UTC 2014
>
> Modified Files:
> src/external/bsd/dhcp/dist/common: ns_name.c
>
> Log Message:
> fix incorrect overflow test: 
> https://android-review.googlesource.com/#/c/50570/

compile failed.

-
/usr/src/external/bsd/dhcp/lib/common/../../dist/common/ns_name.c: In function '
MRns_name_unpack':
/usr/src/external/bsd/dhcp/lib/common/../../dist/common/ns_name.c:348:27: error:
 expected expression before '/' token
if (n >= eom - msg) {  / Out of range. */
   ^
*** [ns_name.o] Error code 1
-

Regards,
-- 
NONAKA Kimihiro


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-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: xsrc/external/mit/xorg-server/dist/hw/netbsd/x68k

2014-03-06 Thread Izumi Tsutsui
> On Tue, Mar 04, 2014 at 12:11:59PM +, Izumi Tsutsui wrote:
> > Module Name:xsrc
> > Committed By:   tsutsui
> > Date:   Tue Mar  4 12:11:59 UTC 2014
> > 
> > Modified Files:
> > xsrc/external/mit/xorg-server/dist/hw/netbsd/x68k: x68kConfig.c
> > x68kGraph.c
> > 
> > Log Message:
> > Replace xalloc(), xrealloc() and xfree() with malloc(), realloc() and 
> > free().
> 
> Lack of error checking?

http://nxr.netbsd.org/xref/xsrc/external/mit/xorg-server/dist/include/os.h?r=1.6#76
http://nxr.netbsd.org/xref/xsrc/external/mit/xorg-server/dist/os/utils.c?r=1.6#1038

No luck anyway.

---
Izumi Tsutsui


Re: CVS commit: src/sys/kern

2014-03-06 Thread Alan Barrett

On Wed, 05 Mar 2014, Andreas Gustafsson wrote:
Changing the types of existing sysctl variables breaks both 
source and binary compatibility and should not be done lightly;


Changing the types without sufficient care can break source 
and binary compatibility.  With sufficient care, compatibility 
can be maintained, and I thought that dsl had attempted to do 
that.  However, I think that a change like that should have been 
discussed first.


that's why we have both both hw.physmem and hw.physmem64, for 
example.


I think that it was a mistake (made several years ago) to 
introduce hw.physmem64, and that hw.physmem should have been 
extended to allow larger values, in a backward compatible way, 
very much like whay dsl has attempted to do now.  Some details 
might be wrong, and it should have been discussed first, but the 
principle of returning what the caller expects seems reasonable to 
me.


I believe the harm caused by the incompatible type change far 
outweighs the cost of a few added lines of code, and would like 
the original types to be restored.


I agree that an incompatible type change would be harmful.  If 
that problem exists, then it could be fixed either by changing the 
types back, or by fixing the compatibility.  Without knowing the 
reason for the type change, I don't know which of those options I 
prefer in the long term.  For the short term, read on.



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, 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.  IMO, 
this behavior violates both the letter of the sysctl() man page 
and the principle of least astonishment.


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.


Yes, I think you are right about the details, and we should 
probably revert the change, at least until a design is discussed 
that meets all reasonable requirements for compatibility.


--apb (Alan Barrett)