tty, hvc, xen: leak? dead code? everything is fine?

tty, hvc, xen: leak? dead code? everything is fine?

[Jesper Juhl]

Hi

Ok, so I'm staring at drivers/tty/hvc/hvc_xen.c::xen_hvm_console_init() - 
to be specific, this code (and yes, it's late and I'm tired, so I may be 
completely off, I admit that up front):

...
	info = vtermno_to_xencons(HVC_COOKIE);
	if (!info) {
		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | 
__GFP_ZERO);
		if (!info)
			return -ENOMEM;
	}

	/* already configured */
	if (info->intfw != NULL)
		return 0;
...

and either I'm completely overlooking something obvious or just being 
stupid (which is certainly possible) or there is a problem here;

If 'info->intfw' is something else than NULL here, then it seems like 
we'll leak the memory we allocated for 'info' when we return (in case 
info started out as NULL and we just allocated it with kzalloc()).

So it seems like we should 'kfree(info)' before the 'return 0' statement, 
but then again, we may not have entered the true branch of 'if (!info)' 
and just allocated 'info' - would it still be right to free it in that 
case?

Or *if* there's actually no way 'info->intfw' can actually be anything but 
NULL in this case, then this would just be dead code (but that doesn't 
seem to be the case).

At the very least, this looks fishy to me... Any feedback is welcome...


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

Re: tty, hvc, xen: leak? dead code? everything is fine?

[Samuel Thibault]

Jesper Juhl, le Thu 12 Apr 2012 23:33:20 +0200, a écrit :
> Or *if* there's actually no way 'info->intfw' can actually be anything but 
> NULL in this case,

There is no way that it's both non-NULL and the if was entered above: if
the if was entered, the content of info is zeroes (kzalloc).

Samuel

Re: tty, hvc, xen: leak? dead code? everything is fine?

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

On Thu, 12 Apr 2012, Samuel Thibault wrote:
> Jesper Juhl, le Thu 12 Apr 2012 23:33:20 +0200, a écrit :
> > Or *if* there's actually no way 'info->intfw' can actually be anything but 
> > NULL in this case,
> 
> There is no way that it's both non-NULL and the if was entered above: if
> the if was entered, the content of info is zeroes (kzalloc).
 
Yep. Thanks Samuel for the quick answer.
 
In more details: if the console has already been configured info->intf
is not NULL.
On the other hand if the console hasn't been configured yet info->intf
is NULL.

If the console has just been allocated by the kzalloc then obviously
it hasn't been configured and in fact info->intf is NULL.