Hi!
On 06/27/2013 05:55 PM, [email protected] wrote:
>
>> +/* lib/tst_run_cmd.c
>> + *
>> + * vfork() + execvp() specified program.
>> + * @argv: a list of two (at least program name + NULL) or more pointers that
>> + * represent the argument list to the new program. The array of pointers
>> + * should be terminated by a NULL pointer.
> must be terminaled by a NULL pointer.
>
> (if it's not, the program will crash)
>
OK
>> +
>> + pid_t pid = vfork();
>> + if (pid == -1) {
>> + tst_brkm(TBROK | TERRNO, cleanup_fn, "vfork failed at %s:%d",
>> + __FILE__, __LINE__);
>> + }
>> + if (!pid)
>> + _exit(execvp(argv[0], argv));
>> +
>> + int ret = -1;
>> + waitpid(pid,&ret, 0);
>> +
>> + if (ret != 0) {
>> + tst_brkm(TBROK, cleanup_fn, "failed to exec cmd '%s' at %s:%d",
>> + argv[0], __FILE__, __LINE__);
>> + }
>> +}
> This version looks nearly fine. I would check the return value from
> waitpid() too, just for the sake of completness.
>
So, something like this is ok:
if (waitpid(pid, &ret, 0) != pid)
tst_brkm(..."waitpid failed at ...")
> And looking at the waitpid() documentation it states that it uses only
> two bytes from ret (in reality it's cleared to zero but that is not
> promised in specification).
>
> I would change the last condition to:
>
> if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
>
> }
But the waitpid() manual says that WEXITSTATUS should only be employed
if WIFEXITED returned true, so may be change it to:
if (!WIFEXITED(ret) || (WIFEXITED(ret) && WEXITSTATUS(ret) != 0)) {
...
}
Thanks,
Alexey
------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:
Build for Windows Store.
http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
