Re: CVS commit: src/external/bsd/atf/dist
On 11/16/11 3:11 PM, Christos Zoulas wrote: In article<4ec40d98.4070...@netbsd.org>, Julio Merino wrote: I know there is no portable way, but at least we can default to "do nothing" if this is not supported. It's better than "not building" :-P Oh, I can arrange that. #ifdef __NetBSD__ :-) But in my view this is worse... Yeah, leave it as is for now. I'll have to do something similar anyway though, but if I can implement the code for the most common systems, that's probably good enough. Yes, they are small because this is just one change. But more may come. I'll then have to go and rewrite all these local changes with portability in mind. When the time to integrate a new release comes, I'll have to mess around with lots merge conflicts, because the upstream code will look nothing like what we have (hence why I asked for this to be reviewed first). Well, this is the world we live in. Next time be the one to upgrade gdb or binutils or ssh and have to deal with 10-50K lines of diffs. You will not be complaining about a 100 line conflict after that :-) That's true... but do we have a chance of getting things integrated into gdb "quickly" and/or on our own to prevent those 10-50K diffs? For atf we do ;-) (OK, I can definitely try to improve on the "quickly" side...) Anyway, just something to keep in mind. Still, I'd appreciate running changes for review at least. I'm open to "overlooking" details like these that don't fit upstream if that's going to make things easier temporarily.
Re: CVS commit: src/external/bsd/atf/dist
In article <4ec40d98.4070...@netbsd.org>, Julio Merino wrote: > >I know there is no portable way, but at least we can default to "do >nothing" if this is not supported. It's better than "not building" :-P Oh, I can arrange that. #ifdef __NetBSD__ :-) But in my view this is worse... >Yes, they are small because this is just one change. But more may come. > I'll then have to go and rewrite all these local changes with >portability in mind. When the time to integrate a new release comes, >I'll have to mess around with lots merge conflicts, because the upstream >code will look nothing like what we have (hence why I asked for this to >be reviewed first). Well, this is the world we live in. Next time be the one to upgrade gdb or binutils or ssh and have to deal with 10-50K lines of diffs. You will not be complaining about a 100 line conflict after that :-) >Of course, if we assume I keep good track of all local changes and >integrate them upstream (I do try), I could ignore the local changes >altogether during the conflicts resolution and use the upstream >copies... but that's... dangerous because I can miss some little thing. > Specially if the local changes are made without tests, because then >it'll be impossible for me to spot when such changes are not preserved. I understand, and when I get more time I will write tests. I just wanted to stop our tests from failing in a non-hacky way quickly and I have achieved my goal with less than 30 minutes of coding. christos
Re: CVS commit: src/external/bsd/atf/dist
On 11/16/11 1:51 PM, Christos Zoulas wrote: In article<4ec3f8f7.8020...@netbsd.org>, Julio Merino wrote: On 11/16/11 12:46 PM, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Wed Nov 16 17:46:16 UTC 2011 Modified Files: src/external/bsd/atf/dist/atf-c++/detail: text.cpp text.hpp src/external/bsd/atf/dist/atf-run: requirements.cpp test-program.cpp Log Message: PR/45619: jmmv: Allow atf tests to request a minimum amount of memory Could you run intrusive changes to atf for review please? The current changes are incomplete and inappropriate, so they are going to make integration upstream a pain. Well, there is really no portable way to find the total available memory of the system that I know of and I did not want to add ifdefs or machinery to get this working on other OS's. If you know otherwise, feel free to fix it. I know there is no portable way, but at least we can default to "do nothing" if this is not supported. It's better than "not building" :-P The changes are really small and they are additions only. I fail to see how it is going to be difficult to integrate. Yes, they are small because this is just one change. But more may come. I'll then have to go and rewrite all these local changes with portability in mind. When the time to integrate a new release comes, I'll have to mess around with lots merge conflicts, because the upstream code will look nothing like what we have (hence why I asked for this to be reviewed first). Of course, if we assume I keep good track of all local changes and integrate them upstream (I do try), I could ignore the local changes altogether during the conflicts resolution and use the upstream copies... but that's... dangerous because I can miss some little thing. Specially if the local changes are made without tests, because then it'll be impossible for me to spot when such changes are not preserved.
Re: CVS commit: src/external/bsd/atf/dist
In article <4ec3f8f7.8020...@netbsd.org>, Julio Merino wrote: >On 11/16/11 12:46 PM, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Wed Nov 16 17:46:16 UTC 2011 >> >> Modified Files: >> src/external/bsd/atf/dist/atf-c++/detail: text.cpp text.hpp >> src/external/bsd/atf/dist/atf-run: requirements.cpp test-program.cpp >> >> Log Message: >> PR/45619: jmmv: Allow atf tests to request a minimum amount of memory > >Could you run intrusive changes to atf for review please? > >The current changes are incomplete and inappropriate, so they are going >to make integration upstream a pain. Well, there is really no portable way to find the total available memory of the system that I know of and I did not want to add ifdefs or machinery to get this working on other OS's. If you know otherwise, feel free to fix it. The changes are really small and they are additions only. I fail to see how it is going to be difficult to integrate. The only difficulty here is to make them portable across OS's, and that can be done with a getrealmemory() stub function per OS, and include the sources for humanize_number for the ones that don't have it. christos
Re: CVS commit: src/tests/lib/libc/regex
On Nov 16, 10:23am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/tests/lib/libc/regex | Since this test is succeeding on amd64 with only 128MB allocated in | qemu, do we really need to set the minimum to 500? Ok, I will reduce it. christos
Re: CVS commit: src/tests/lib/libc/regex
Since this test is succeeding on amd64 with only 128MB allocated in qemu, do we really need to set the minimum to 500? On Wed, 16 Nov 2011, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Wed Nov 16 17:47:19 UTC 2011 Modified Files: src/tests/lib/libc/regex: t_exhaust.c Log Message: add require.memory=500M To generate a diff of this commit: cvs rdiff -u -r1.5 -r1.6 src/tests/lib/libc/regex/t_exhaust.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. !DSPAM:4ec3f72b1961213312912! - | 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/external/bsd/atf/dist
On 11/16/11 12:46 PM, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Wed Nov 16 17:46:16 UTC 2011 Modified Files: src/external/bsd/atf/dist/atf-c++/detail: text.cpp text.hpp src/external/bsd/atf/dist/atf-run: requirements.cpp test-program.cpp Log Message: PR/45619: jmmv: Allow atf tests to request a minimum amount of memory Could you run intrusive changes to atf for review please? The current changes are incomplete and inappropriate, so they are going to make integration upstream a pain. +int64_t +impl::to_number(const std::string& str) +{ + int64_t num; + ::dehumanize_number(str.c_str(),&num); This adds a dependency on NetBSD that should not be there. [...] +static +std::string +check_memory(const std::string& memory) +{ +// Make sure we have enough memory +int64_t memneed = atf::text::to_number(memory); +int64_t memavail; +size_t len = sizeof(memavail); + +if (::sysctlbyname("hw.usermem64",&memavail,&len, NULL, 0) == -1) { Same here and below in the same function. This is NetBSD specific and there is no provisions to support other systems. [...] Lastly, this has been added without absolutely no tests and no documentation. (And there seem to be whitespace issues, although I can't tell if these are my mua's fault...)
Re: CVS commit: src/tests/util/sh
On 11/14/11 3:23 PM, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Mon Nov 14 20:23:29 UTC 2011 Modified Files: src/tests/util/sh: Makefile Added Files: src/tests/util/sh: t_evaltested.sh Log Message: Add a test for PR/45613 (eval failing in a tested context) This looks like a poor name for the program: in general, if the test program contains a single test case and both have the same name, that's a bad symptom. Following prior art in this same directory, the program should have probably been named "t_eval" and the single test that it currently has "tested_context". This would leave room for the future addition of eval-related tests, whereas the current naming does not.
Re: CVS commit: src/lib/libc/regex
do we have to fix src/dist/nvi/regex too? it is same spencer regex code as src/lib/libc/regex (but modified for wide character). ftp://ftp.netbsd.org/pub/NetBSD/misc/tnozaki/patch-dist_nvi_regex very truly yours -- Takehiko NOZAKI 2011/10/10 Christos Zoulas : > Module Name: src > Committed By: christos > Date: Sun Oct 9 18:23:00 UTC 2011 > > Modified Files: > src/lib/libc/regex: engine.c regcomp.c regex2.h > > Log Message: > Prevent regcomp/regexec DoS attacks by limiting the amount of memory used > and the level of recursion. Thanks to Maksymilian Arciemowicz for discovery > and help with the implementation. > > > To generate a diff of this commit: > cvs rdiff -u -r1.22 -r1.23 src/lib/libc/regex/engine.c > cvs rdiff -u -r1.29 -r1.30 src/lib/libc/regex/regcomp.c > cvs rdiff -u -r1.12 -r1.13 src/lib/libc/regex/regex2.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > >