[patch iwhd 2/3] do not leak "pending"

Jim Meyering jim at meyering.net
Thu Jul 7 13:27:39 UTC 2011


Pete Zaitcev wrote:

> Currently we set "pending" in case anything goes wrong, we crash or
> whatnot, so that users can know that ami-id may be invalid. This is
> fine, but if the context of POST is lost, it is not possible to
> figure out what is going on: either the registration is still in
> progress, or it is failed. Of course if we actually crash after
> setting "pending", then it's bad. But we also leave "penging"

Hi Pete,

s/penging/pending/?

> dangling for any error exit of dc-rhev-image, and that is wrong.
>
> So, plug the return paths where "pending" may remain set after
> everything is over. Not that this patch is not complete: the waitpid
> is going to be fixed over with another patch.

Thanks for the RM#1888 reference.
However, it's better to include that in the commit log,
and to include an actual URL, so that it's easier to find:

    http://www.aeolusproject.org/redmine/issues/1888


> ---
>  backend.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> RM#1888.
>
> commit fa7a328130d519fe54c5f21898dc0918a9a6a847
> Author: Pete Zaitcev <zaitcev at yahoo.com>
> Date:   Wed Jul 6 16:52:37 2011 -0600
>
>     Suppress leaks of "pending".
>
> diff --git a/backend.c b/backend.c
> index cd1a685..27f9365 100644
> --- a/backend.c
> +++ b/backend.c
> @@ -1546,6 +1546,7 @@ fs_rhevm_register (my_state *ms, const provider_t *prov, const char *next,
>
>  	if (pipe(organ) < 0) {
>  		error (0, errno, _("pipe creation failed"));
> +		sprintf(ami_id_buf,"failed (pipe)");

Please use strcpy in cases like this where there is no
sprintf formatting directive:

    		strcpy(ami_id_buf,"failed (pipe)");


>  		goto cleanup;
>  	}
>
> @@ -1554,6 +1555,7 @@ fs_rhevm_register (my_state *ms, const provider_t *prov, const char *next,
>  		error (0, errno, _("fork failed"));
>  		close(organ[0]);
>  		close(organ[1]);
> +		sprintf(ami_id_buf,"failed (fork)");
>  		goto cleanup;
>  	}
>
> @@ -1569,6 +1571,7 @@ fs_rhevm_register (my_state *ms, const provider_t *prov, const char *next,
>  	if (!fp) {
>  		error (0, 0, _("could not open parent pipe stream"));
>  		close(organ[0]);
> +		sprintf(ami_id_buf,"failed (fdopen)");
>  		goto cleanup;
>  	}
>  	while (fgets(buf,sizeof(buf)-1,fp)) {
> @@ -1593,6 +1596,7 @@ fs_rhevm_register (my_state *ms, const provider_t *prov, const char *next,
>  	DPRINTF("waiting for child...\n");
>  	if (waitpid(pid,&wstat,0) < 0) {
>  		error (0, errno, _("waitpid failed"));
> +		sprintf(ami_id_buf,"failed (waitpid)");
>  		goto cleanup;
>  	}
>  	if (!WIFEXITED(wstat)) {
> @@ -1606,6 +1610,14 @@ fs_rhevm_register (my_state *ms, const provider_t *prov, const char *next,
>  	}
>  	DPRINTF("...child exited\n");
>
> +	if (strncmp(ami_id_buf,"pending",sizeof("pending")-1)==0) {
> +		/*
> +		 * Unfortunately "pending" may get stuck if dc-rhev-image
> +		 * bums on us by crashing or otherwise fails to post result.
> +		 */
> +		sprintf(ami_id_buf,"failed (back-end)");
> +	}
> +
>  cleanup:
>  	/*
>  	 * Unfortunately we cannot collect zombies indiscriminately here


More information about the iwhd-devel mailing list