Re: [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen

[Corey Minyard <minyard@acm.org>]

Please run this through checkpatch, as it has coding style violations.

Also, this patch appears to fix bugs in addition to adding the print.  
Can we have a separate patch for that?

I'm also not clear on the reason for this.  I believe all this 
information is already available in /proc/ipmi/<if#>/params.  I don't 
think there is a strong reason to print it to the log.

-corey

On 07/26/2010 08:25 PM, Yinghai Lu wrote:
> also print out the reg spacing and size for spmi and smbios.
> so bios guys could have idea to make them consistent.
>
> Signed-off-by: Yinghai Lu<yinghai@kernel.org>
>
> ---
>   drivers/char/ipmi/ipmi_si_intf.c |   47 ++++++++++++++++++++++++++++++++-------
>   1 file changed, 39 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/ipmi/ipmi_si_intf.c
> +++ linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1804,9 +1804,12 @@ static int hotmod_handler(const char *va
>   				info->irq_setup = std_irq_setup;
>   			info->slave_addr = ipmb;
>
> -			if (!add_smi(info))
> +			if (!add_smi(info)) {
>   				if (try_smi_init(info))
>   					cleanup_one_si(info);
> +			} else {
> +				kfree(info);
> +			}
>   		} else {
>   			/* remove */
>   			struct smi_info *e, *tmp_e;
> @@ -1890,9 +1893,12 @@ static __devinit void hardcode_find_bmc(
>   			info->irq_setup = std_irq_setup;
>   		info->slave_addr = slave_addrs[i];
>
> -		if (!add_smi(info))
> +		if (!add_smi(info)) {
>   			if (try_smi_init(info))
>   				cleanup_one_si(info);
> +		} else {
> +			kfree(info);
> +		}
>   	}
>   }
>
> @@ -2088,7 +2094,13 @@ static __devinit int try_init_spmi(struc
>   	}
>   	info->io.addr_data = spmi->addr.address;
>
> -	add_smi(info);
> +	pr_info("ipmi_si: SPMI: %s %#lx regsize %d spacing %d irq %d\n",
> +		 (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> +		 info->io.addr_data, info->io.regsize, info->io.regspacing,
> +		 info->irq);
> +
> +	if (add_smi(info))
> +		kfree(info);
>
>   	return 0;
>   }
> @@ -2204,7 +2216,10 @@ static int __devinit ipmi_pnp_probe(stru
>   		 res, info->io.regsize, info->io.regspacing,
>   		 info->irq);
>
> -	return add_smi(info);
> +	if (add_smi(info))
> +		goto err_free;
> +
> +	return 0;
>
>   err_free:
>   	kfree(info);
> @@ -2362,7 +2377,13 @@ static __devinit void try_init_dmi(struc
>   	if (info->irq)
>   		info->irq_setup = std_irq_setup;
>
> -	add_smi(info);
> +	pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n",
> +		 (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> +		 info->io.addr_data, info->io.regsize, info->io.regspacing,
> +		 info->irq);
> +
> +	if (add_smi(info))
> +		kfree(info);
>   }
>
>   static void __devinit dmi_find_bmc(void)
> @@ -2468,7 +2489,10 @@ static int __devinit ipmi_pci_probe(stru
>   		&pdev->resource[0], info->io.regsize, info->io.regspacing,
>   		info->irq);
>
> -	return add_smi(info);
> +	if (add_smi(info))
> +		kfree(info);
> +
> +	return 0;
>   }
>
>   static void __devexit ipmi_pci_remove(struct pci_dev *pdev)
> @@ -2581,7 +2605,12 @@ static int __devinit ipmi_of_probe(struc
>
>   	dev_set_drvdata(&dev->dev, info);
>
> -	return add_smi(info);
> +	if (add_smi(info)) {
> +		kfree(info);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
>   }
>
>   static int __devexit ipmi_of_remove(struct of_device *dev)
> @@ -3018,6 +3047,8 @@ static __devinit void default_find_bmc(v
>   				info->io.addr_data);
>   			} else
>   				cleanup_one_si(info);
> +		} else {
> +			kfree(info);
>   		}
>   	}
>   }
> @@ -3045,7 +3076,7 @@ static int add_smi(struct smi_info *new_
>   			si_to_str[new_smi->si_type]);
>   	mutex_lock(&smi_infos_lock);
>   	if (!is_new_interface(new_smi)) {
> -		printk(KERN_CONT PFX "duplicate interface\n");
> +		printk(KERN_CONT " duplicate interface\n");
>   		rv = -EBUSY;
>   		goto out_err;
>   	}
>