From: Garrett C. <yan...@gm...> - 2010-07-24 00:56:12
|
On Fri, Jul 23, 2010 at 4:20 PM, Nicolas Joly <nj...@pa...> wrote: > On Fri, Jul 23, 2010 at 03:03:45PM -0700, Garrett Cooper wrote: >> On Jul 23, 2010, at 11:48 AM, Nicolas Joly wrote: >> >> > On Fri, Jul 23, 2010 at 10:56:58AM -0700, Garrett Cooper wrote: >> >> On Fri, Jul 23, 2010 at 10:17 AM, Nicolas Joly <nj...@pa...> wrote: >> >>> >> >>> Hi, >> >>> >> >>> The attached patch do some cleanup in the abort01 testcase ... >> >>> >> >>> 1) The attempt to remove the generated core is wrong. The hard-coded >> >>> `core' name is bad and the test temporary directory removal already >> >>> take care of this. >> >>> >> >>> 2) Make the test fail gracefully if the running environment does not >> >>> allow generating core files. >> >>> >> >>> njoly@lanfeust [syscalls/abort]> ./abort01 >> >>> abort01 1 TPASS : Test passed >> >>> njoly@lanfeust [syscalls/abort]> (ulimit -c 0 && ./abort01) >> >>> abort01 1 TCONF : core file size limit must be greater than 0. >> >>> >> >>> Signed-off-by: Nicolas Joly <nj...@pa...> >> >> >> >> Looks ok, but is there any particular reason why you removed the unlink(2) call? >> > >> > The `unlink("core")' one ? >> > >> > Well, at least RHEL do use `proc.<pid>' when generating core file >> > names ... Not speaking about admins that might have configured it to >> > use another scheme. >> > >> > IMO, relying on a specific name, for a configurable system, is a bad >> > idea; and can only lead to problems. >> > >> > By example, this test from mkdir09.c cannot succeed on most RHEL >> > systems: >> > >> > /* Check for core file in test directory. */ >> > if (access("core", 0) == 0) { >> > tst_resm(TWARN, "\tCore file found in test directory."); >> > tst_exit(); >> > } >> >> Yeah. Perhaps it might be a good idea to integrate a tool into LTP which detects corefile names, like what's described here: http://aplawrence.com/Linux/limit_core_files.html . > > If there is a real need, why not; but for the current use in syscall > testcases, it seems a little overkill. > > testcases/kernel/syscalls/abort/abort01.c > testcases/kernel/syscalls/kill/kill11.c > testcases/kernel/syscalls/mallopt/mallopt01.c > testcases/kernel/syscalls/waitpid/waitpid02.c > testcases/kernel/syscalls/waitpid/waitpid05.c > Remove core files that may have been created during the test > ... The temp test directory removal can/do already remove them. > > testcases/kernel/syscalls/mkdir/mkdir09.c > testcases/kernel/syscalls/setrlimit/setrlimit01.c > Check that a childs does not have created a core file > ... Could be tested from child process status instead. It's not required to fix this testcase (and I'll commit this once I get back home), but it really should be added. Thanks! -Garrett |