On Fri, Jul 23, 2010 at 4:20 PM, Nicolas Joly <njoly@...> 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 <njoly@...> 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@... [syscalls/abort]> ./abort01
>> >>> abort01 1 TPASS : Test passed
>> >>> njoly@... [syscalls/abort]> (ulimit -c 0 && ./abort01)
>> >>> abort01 1 TCONF : core file size limit must be greater than 0.
>> >>>
>> >>> Signed-off-by: Nicolas Joly <njoly@...>
>> >>
>> >> 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
|