[Rpm-maint] PATCH 2/2013 Add missing header against WEXITSTATUS

[Panu Matilainen]

On 08/17/2013 07:02 PM, Kamil Rytarowski wrote:
> Hello,
>
> I'm attaching a patch with the following change-log:
> Add missing header against WEXITSTATUS()
>
> WEXITSTATUS is defined in sys/wait.h.
>
> Please apply the patch.

Sure, applied. Thanks for the patch.

>
> As a side note this code (build/pack.c) looks suspicious:
>
> static rpmRC checkPackages(char *pkgcheck)
> {
>   int fail = rpmExpandNumeric("%{?_nonzero_exit_pkgcheck_terminate_build}");
>   int xx;
>
>   rpmlog(RPMLOG_NOTICE, _("Executing \"%s\":\n"), pkgcheck);
>   xx = system(pkgcheck);
>   if (WEXITSTATUS(xx) == -1 || WEXITSTATUS(xx) == 127) {
> rpmlog(RPMLOG_ERR, _("Execution of \"%s\" failed.\n"), pkgcheck);
> if (fail) return RPMRC_NOTFOUND;
>   }
>   if (WEXITSTATUS(xx) != 0) {
> rpmlog(RPMLOG_ERR, _("Package check \"%s\" failed.\n"), pkgcheck);
> if (fail) return RPMRC_FAIL;
>   }
>
>   return RPMRC_OK;
> }
>
> 1. We lost errno.
> 2. WEXITSTATUS(xx) == -1 does the standard guarantee that this evaluates on all POSIX Unices as true? From what I saw in Unix manuals [1] here the result status is evaluated to low-order byte, so it would be better to compare it with 255 (to ignore signed/unsigned conversions), or better: to stop using WEXITSTATUS macro for this particular scenario and compare xx as it is with -1.

Yup, the code does look somewhat suspicious. Patches welcome :)

	- Panu -

> [1] like http://modman.unixdev.net/?sektion=2&page=WEXITSTATUS&manpath=SunOS-4.1.3