Re: CVS commit: src/sys/sys
hi, > In article <20120318191308.ga10...@britannica.bec.de>, > Joerg Sonnenberger wrote: >>On Sat, Mar 17, 2012 at 05:30:31PM -0400, Christos Zoulas wrote: >>> Module Name:src >>> Committed By: christos >>> Date: Sat Mar 17 21:30:30 UTC 2012 >>> >>> Modified Files: >>> src/sys/sys: types.h >>> >>> Log Message: >>> PR/44847: Jukka Ruohonen: blksize_t should be signed. >>> http://pubs.opengroup.org/onlinepubs/95399/basedefs/sys/types.h.html >> >>I dislike the change. What is the justification for requiring this to be >>signed? There are good reasons for having it be unsigned, e.g. getting >>more efficient code by default. > > va_blocksize is signed (long) and most of the 360+ userland occurances > assume it is signed. where did you count the number? the occurrences in our tree doesn't seem so many. YAMAMOTO Takashi > > christos
Build failure on port amd64 - tests/vfs/t_vnops.o
Module Name:src
Committed By: uebayasi
Date: Mon Mar 19 00:17:08 UTC 2012
Modified Files:
src/sys/uvm: uvm_param.h
Log Message:
Expose vm_inherit/voff_t/pgoff_t to userland to fix build.
Even with this change, I'm still seeing the following errors while
compiling the vfs ATF tests.
# compile vfs/t_vnops.o
/build/netbsd-local/tools/x86_64/amd64/bin/x86_64--netbsd-gcc -O2 -std=gnu99
-Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith
-Wno-sign-compare -Wno-traditional -Wa,--fatal-warnings -Wreturn-type
-Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wextra -Wno-unused-parameter
-Wno-sign-compare -Wsign-compare -Wformat=2 -Wno-format-zero-length -Werror
-Wno-missing-noreturn --sysroot=/build/netbsd-local/dest/amd64 -c
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c:323:3: error: expected
identifier or '(' before '/' token
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c:327:17: error: expected
declaration specifiers or '...' before string constant
cc1: warnings being treated as errors
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c:327:2: error: data definition
has no type or storage class
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c:327:2: error: type defaults to
'int' in declaration of 'rump_sys_chdir'
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c:327:17: error: function
declaration isn't a prototype
/build/netbsd-local/src/tests/fs/vfs/t_vnops.c:328:1: error: expected
identifier or '(' before '}' token
-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-
Re: CVS commit: src/sys/sys
In article <20120318213646.ga13...@britannica.bec.de>, Joerg Sonnenberger wrote: >That doesn't say anything about why it should be signed. It is just >another place where signed is used for no good reason. Yes, it just explains that making unsigned is a larger impact change and it would violate the standard. christos
Re: CVS commit: src
On Mar 18, 2012, at 2:08 PM, Martin Husemann wrote: > On Sun, Mar 18, 2012 at 09:54:31AM -0700, Matt Thomas wrote: >> We probably should have a -Os compiled libc (without _DIAGASSERT) >> for use by distrib. > > Or an empty __diagassert13() stub in distrib/utils/libhack. Not enough. _DIAGASSERT causes about 50KB of bloat, much more than __diagassert13 alone.
Re: CVS commit: src/sys/sys
On Sun, Mar 18, 2012 at 09:00:54PM +, Christos Zoulas wrote: > In article <20120318191308.ga10...@britannica.bec.de>, > Joerg Sonnenberger wrote: > >On Sat, Mar 17, 2012 at 05:30:31PM -0400, Christos Zoulas wrote: > >> Module Name: src > >> Committed By: christos > >> Date: Sat Mar 17 21:30:30 UTC 2012 > >> > >> Modified Files: > >>src/sys/sys: types.h > >> > >> Log Message: > >> PR/44847: Jukka Ruohonen: blksize_t should be signed. > >> http://pubs.opengroup.org/onlinepubs/95399/basedefs/sys/types.h.html > > > >I dislike the change. What is the justification for requiring this to be > >signed? There are good reasons for having it be unsigned, e.g. getting > >more efficient code by default. > > va_blocksize is signed (long) and most of the 360+ userland occurances > assume it is signed. That doesn't say anything about why it should be signed. It is just another place where signed is used for no good reason. Joerg
Re: CVS commit: src
On Sun, Mar 18, 2012 at 09:54:31AM -0700, Matt Thomas wrote: > We probably should have a -Os compiled libc (without _DIAGASSERT) > for use by distrib. Or an empty __diagassert13() stub in distrib/utils/libhack. Martin
Re: CVS commit: src/sys/sys
In article <20120318191308.ga10...@britannica.bec.de>, Joerg Sonnenberger wrote: >On Sat, Mar 17, 2012 at 05:30:31PM -0400, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Sat Mar 17 21:30:30 UTC 2012 >> >> Modified Files: >> src/sys/sys: types.h >> >> Log Message: >> PR/44847: Jukka Ruohonen: blksize_t should be signed. >> http://pubs.opengroup.org/onlinepubs/95399/basedefs/sys/types.h.html > >I dislike the change. What is the justification for requiring this to be >signed? There are good reasons for having it be unsigned, e.g. getting >more efficient code by default. va_blocksize is signed (long) and most of the 360+ userland occurances assume it is signed. christos
Re: CVS commit: src/lib/libc/sys
On Sun, Mar 18, 2012 at 08:37:20PM +0200, Jukka Ruohonen wrote:
> On Sat, Mar 17, 2012 at 10:04:40PM -0400, Christos Zoulas wrote:
> > Module Name:src
> > Committed By: christos
> > Date: Sun Mar 18 02:04:40 UTC 2012
> >
> > Modified Files:
> > src/lib/libc/sys: sched.c
> >
> > Log Message:
> > fail as the man page says sched_rr_get_interval should.
>
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -123,6 +124,8 @@ int
> sched_rr_get_interval(pid_t pid, struct timespec *interval)
> {
>
> + if (pid && kill(pid, 0) == -1)
> + return -1;
> interval->tv_sec = 0;
> interval->tv_nsec = sysconf(_SC_SCHED_RT_TS) * 1000;
> return 0;
>
> So to return to this: surely this can't be right?
>From the kill man page:
sig may be one of the signals specified in sigaction(2) or it may be
0, in which case error checking is performed but no signal is actually
sent.
Thomas
Re: CVS commit: src/sys/sys
On Sat, Mar 17, 2012 at 05:30:31PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sat Mar 17 21:30:30 UTC 2012 > > Modified Files: > src/sys/sys: types.h > > Log Message: > PR/44847: Jukka Ruohonen: blksize_t should be signed. > http://pubs.opengroup.org/onlinepubs/95399/basedefs/sys/types.h.html I dislike the change. What is the justification for requiring this to be signed? There are good reasons for having it be unsigned, e.g. getting more efficient code by default. Joerg
Re: CVS commit: src/lib/libc/sys
On Sat, Mar 17, 2012 at 10:04:40PM -0400, Christos Zoulas wrote:
> Module Name: src
> Committed By: christos
> Date: Sun Mar 18 02:04:40 UTC 2012
>
> Modified Files:
> src/lib/libc/sys: sched.c
>
> Log Message:
> fail as the man page says sched_rr_get_interval should.
#include
#include
#include
#include
+#include
#include
#include
@@ -123,6 +124,8 @@ int
sched_rr_get_interval(pid_t pid, struct timespec *interval)
{
+ if (pid && kill(pid, 0) == -1)
+ return -1;
interval->tv_sec = 0;
interval->tv_nsec = sysconf(_SC_SCHED_RT_TS) * 1000;
return 0;
So to return to this: surely this can't be right?
- Jukka.
Re: CVS commit: src/sbin/ifconfig
On Sun, Mar 18, 2012 at 05:08:44PM +0200, Jukka Ruohonen wrote: > All good; exited without nothing printed to stderr and the status is zero. > But this: > > atf_check -o ignore \ > -s exit:0 -x "find /etc -type f -exit" > > yields > > tc-so:Executing command [ /bin/sh -c find /etc -type f -exit ] > tc-se:Fail: incorrect exit status: 1, expected: 0 > tc-se:stdout: > tc-se: > tc-se:stderr: > tc-se:find: fts_read: Inappropriate ioctl for device > tc-se: > tc-end: 1332083101.965143, exit, failed, atf-check failed; > see the output of the test for details > > So what is going on here? As njoly posted in the PR it looks as if redirecting stdin from /dev/null causes something to be left in errno that find then prints as per the PR. With the fix I just committed it doesn't do this, so I think we can (1) declare the PR fixed and (2) conclude that the behavior you're seeing isn't atf's fault. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src
On Mar 18, 2012, at 7:12 AM, Havard Eidnes wrote: > Module Name: src > Committed By: he > Date: Sun Mar 18 14:12:56 UTC 2012 > > Modified Files: > src/distrib/vax/ramdisk: Makefile > src/sys/arch/vax/conf: INSTALL > > Log Message: > Bump the ramdisk size so that the contents fits again. That really wasn't the best way to fix that. The problem is _DIAGASSERT in libc bloated the size. We probably should have a -Os compiled libc (without _DIAGASSERT) for use by distrib.
Re: CVS commit: src/sbin/ifconfig
On Sun, Mar 18, 2012 at 11:57:08PM +0900, Julio Merino wrote: > >But why is stderr being involved? > > If you really want to ignore stderr in the test, pass "-e ignore" to > atf_check. The default is "-o empty -e empty". > > Now, the question is: do you really want to ignore the message, or do > you want to validate that the appropriate error message is printed? If > the later (which should be the preferred way), "-e inline:'foo bar\n'" > or "-e match:'foo bar'" or "-e file:experr" (with experr being > pre-populated with the expected message) will do the job. Thanks; this was pointed out already. But I have a related question. Supposedly ATF uses something like freopen(3) to capture the output in the standard streams. Now consider this from one test I just added: $ find /etc -type f -exit $ echo $? 0 All good; exited without nothing printed to stderr and the status is zero. But this: atf_check -o ignore \ -s exit:0 -x "find /etc -type f -exit" yields tc-so:Executing command [ /bin/sh -c find /etc -type f -exit ] tc-se:Fail: incorrect exit status: 1, expected: 0 tc-se:stdout: tc-se: tc-se:stderr: tc-se:find: fts_read: Inappropriate ioctl for device tc-se: tc-end: 1332083101.965143, exit, failed, atf-check failed; see the output of the test for details So what is going on here? Likewise $ find / -exit 99 $ echo $? 99 contra atf_check -o ignore -e ignore -s exit:99 -x "find / -exit 99" and tc-so:Executing command [ /bin/sh -c find / -exit 99 ] tc-se:Fail: incorrect exit status: 1, expected: 99 tc-se:stdout: tc-se: tc-se:stderr: tc-se:find: fts_read: Inappropriate ioctl for device tc-se: tc-end: 1332083172.374247, exit_status, failed, atf-check failed; see the output of the test for details - Jukka.
Re: CVS commit: src/sbin/ifconfig
On 3/18/12 1:53 AM, Jukka Ruohonen wrote: On Fri, Mar 16, 2012 at 10:25:08PM -0400, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sat Mar 17 02:25:08 UTC 2012 Modified Files: src/sbin/ifconfig: af_inetany.c Log Message: PR/43141: Tobias Nygren: Print an error on unknown interfaces. Now this is trival and it is fixed. But I guess there is a bug here in ATF. The test only checks that the exit code is not zero: atf_check -s not-exit:0 ifconfig nonexistent0 1.2.3.4/24 But why is stderr being involved? If you really want to ignore stderr in the test, pass "-e ignore" to atf_check. The default is "-o empty -e empty". Now, the question is: do you really want to ignore the message, or do you want to validate that the appropriate error message is printed? If the later (which should be the preferred way), "-e inline:'foo bar\n'" or "-e match:'foo bar'" or "-e file:experr" (with experr being pre-populated with the expected message) will do the job.
Re: CVS commit: [netbsd-6] src/etc/root
Hello,
Bernd Ernesti writes:
> Hi Havard,
>
> see the issue at the end.
>
> On Sun, Mar 18, 2012 at 01:20:02AM +0400, Aleksej Saushev wrote:
>> "Manuel Bouyer" writes:
>>
>> > Module Name: src
>> > Committed By: bouyer
>> > Date: Sat Mar 17 17:22:54 UTC 2012
>> >
>> > Modified Files:
>> >src/etc/root [netbsd-6]: dot.cshrc dot.profile
>> >
>> > Log Message:
>> > Pull up following revision(s) (requested by he in ticket #109):
>> >etc/root/dot.cshrc: revision 1.23
>> >etc/root/dot.cshrc: revision 1.24
>> >etc/root/dot.profile: revision 1.26
>> > Can't use Bourne shell syntax here... (Even in comments, for PKG_PATH.)
>> > Point first to 6.0 packages, then to packages for 5.1 or 5.0.
>> > The latter have reduced usefullness in -current or netbsd-6 until
>> > we have a compat50 package.
>> >
>> >
>> > To generate a diff of this commit:
>> > cvs rdiff -u -r1.22 -r1.22.4.1 src/etc/root/dot.cshrc
>> > cvs rdiff -u -r1.25 -r1.25.4.1 src/etc/root/dot.profile
>> >
>> > Please note that diffs are not public domain; they are subject to the
>> > copyright notices on the relevant files.
>> >
>> >
>> > Modified files:
>> ...
>> > Index: src/etc/root/dot.profile
>> > diff -u src/etc/root/dot.profile:1.25 src/etc/root/dot.profile:1.25.4.1
>> > --- src/etc/root/dot.profile:1.25 Tue Jun 21 05:31:29 2011
>> > +++ src/etc/root/dot.profile Sat Mar 17 17:22:54 2012
>> > @@ -1,11 +1,12 @@
>> > -# $NetBSD: dot.profile,v 1.25 2011/06/21 05:31:29 erh Exp $
>> > +# $NetBSD: dot.profile,v 1.25.4.1 2012/03/17 17:22:54 bouyer Exp $
>> >
>> > export PATH=/sbin:/usr/sbin:/bin:/usr/bin:/usr/pkg/sbin:/usr/pkg/bin
>> > export
>> > PATH=${PATH}:/usr/X11R7/bin:/usr/X11R6/bin:/usr/local/sbin:/usr/local/bin
>> >
>> > # Uncomment the following line(s) to install binary packages
>> > # from ftp.NetBSD.org via pkg_add. (See also pkg_install.conf)
>> > -#export PKG_PATH="ftp://ftp.NetBSD.org/pub/pkgsrc/packages/NetBSD/$(uname
>> > -m)/5.1/All"
>> > +#export PKG_PATH=ftp://ftp.NetBSD.org/pub/pkgsrc/packages/NetBSD/$(uname
>> > -m)/6.0/All
>> > +#export
>> > PKG_PATH="${PKG_PATH};ftp://ftp.NetBSD.org/pub/pkgsrc/packages/NetBSD/$(uname
>> > -m)/5.1/All"
>> > #export
>> > PKG_PATH="${PKG_PATH};ftp://ftp.NetBSD.org/pub/pkgsrc/packages/NetBSD/$(uname
>> > -m)/5.0/All"
>> >
>> > export BLOCKSIZE=1k
>>
>> Why were quotes dropped?
>
> You are asking the wrong person. That change come from the HEAD commit
> by Havard, this is just the pullup.
I understand it. I must have missed the original commit mail.
I'm just a bit puzzled why it was changed to a different style.
--
HE CE3OH...
