On 04/14/2014 09:00 PM, Sami Kerola wrote:
On 14 April 2014 17:14, Bernhard Voelker <mail@xxxxxxxxxxxxxxxxxxx> wrote:
Most of the tests could use pid, but I am afraid there should be at
least one of them using name. Else killing by name remains untested.
What about creating a test 'kill_by_name', and avoiding complexity
in the other tests? They all have a certain purpose each, so there's
no need to put a certain aspect into all of them.
Of course simple kill by name test could do the job, but I am not
hugely in favor to simplify tests until they work without
understanding why they fail in first place.
Agreed. Unfortunately, I didn't find *the* reason for the failure yet.
BTW, I often see both "all_processes" and "name_to_number" failing here.
It seems that there are several issues.
First of all, this looks odd in the receiver program:
> + if (!stat(argv[1], &statbuf)) {
> + if (S_ISDIR(statbuf.st_mode))
> + xasprintf(&pid_path, "%s/%d", argv[1], getpid());
> + } else
> + xasprintf(&pid_path, "%s", argv[1]);
What if the argv[1] is stat()able but not a directory?
Anyway, I'm 40:60 against letting the receiver program decide where
the witness file is located, i.e. I'd rather use deterministic names
everywhere. E.g. in the 'name_to_number' tests, the name of the witness
files could contain the signal name rather than using unsafe mktemp.
Furthermore, the open() call would return -1 upon failure instead
of 0 as assumed here:
> + if (!(fd = open(pid_path, O_WRONLY | O_CREAT | O_TRUNC, mode)))
> + err(EXIT_FAILURE, "cannot open pid file: %s", pid_path);
Re. the "all_processes" test: I'm not sure how su(1) and the intermediate
shell change the signal handling. Maybe the reason for the failure is
somehow related to this, because $! will refer to the shell's PID rather
than to that of the receiver program.
Anyway, maybe it's better to use $TS_CMD_SU instead of the local 'su',
or even adding a dedicated setuid() helper.
As already said, these are just side notes.
Have a nice day,
Berny
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
