On Tue, Aug 26, 2014 at 12:05:51PM -0700, K. Y. Srinivasan wrote:
> -static void vmbus_close_internal(struct vmbus_channel *channel)
> +static int vmbus_close_internal(struct vmbus_channel *channel)
> {
> struct vmbus_channel_close_channel *msg;
> - int ret;
> + int ret = 0;
GCC has a feature which warns about uninitialized variables. Those
features are there to help prevent bugs. You are turning the feature
off here by initializing it with a bogus value. Don't do that.
>
> channel->state = CHANNEL_OPEN_STATE;
> channel->sc_creation_callback = NULL;
> @@ -502,11 +502,28 @@ static void vmbus_close_internal(struct vmbus_channel *channel)
>
> ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel));
>
> - BUG_ON(ret != 0);
> + if (ret) {
> + pr_err("Close failed: close post msg return is %d\n", ret);
> + /*
> + * If we failed to post the close msg,
> + * it is perhaps better to leak memory.
> + */
> + goto close_err;
Just return directly. Don't introduce do-nothing gotos to lead the
reader through a series of pointless goto hops.
The goto label is poorly chosen. Label names should be based on the
thing which they do. "close_err" implies that something is closed but
that's not the case, the label doesn't do anything.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
