lkml.org 
[lkml]   [2014]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/4] Drivers: hv: vmbus: Cleanup vmbus_close_internal()
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



\
 
 \ /
  Last update: 2014-08-27 12:21    [W:0.082 / U:1.796 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site