Re: transport_class: BUG if we can't release the attribute container

Greg KH
2008-04-02T06:32:56Z

On Sat, Mar 22, 2008 at 11:39:17AM -0500, James Bottomley wrote:
> Every current transport class calls transport_container_release but
> ignores the return value. This is catastrophic if it returns an error
> because the containers are part of a global list and the next action of
> almost every transport class is to free the memory used by the
> container.
>
> Fix this by making transport_container_release a void, but making it BUG
> if attribute_container_release returns an error ... this catches the
> root cause of a system panic much earlier. If we don't do this, we get
> an eventual BUG when the attribute container list notices the corruption
> caused by the freed memory it's still referencing.
>
> Also made attribute_container_release __must_check as a reminder.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> ---
>
> diff --git a/drivers/base/transport_class.c b/drivers/base/transport_class.c
> index 40bca48..06861ae 100644
> --- a/drivers/base/transport_class.c
> +++ b/drivers/base/transport_class.c
> @@ -108,7 +108,7 @@ EXPORT_SYMBOL_GPL(anon_transport_class_register);
> */
> void anon_transport_class_unregister(struct anon_transport_class *atc)
> {
> - attribute_container_unregister(&atc->container);
> + BUG_ON(attribute_container_unregister(&atc->container));

BUG_ON() should not do anything in the macro except test for a value, no
function calling. I think checkpatch.pl checks for this...

If you change that, I have no problem with this.

thanks,

greg k-h