Re: CVS commit: src/external/bsd/dhcp/dist/common
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
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
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
> 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
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)