Re: [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device

[ykzhao <yakui.zhao@intel.com>]

On Fri, 2010-10-15 at 11:03 +0800, Corey Minyard wrote:
> On 10/14/2010 08:07 PM, ykzhao wrote:
> >
> >> The way you are doing it, there is no need for a refcount, since you are
> >> making a copy of the data.
> >>
> >> Is a copy or a pointer better?  A pointer is generally preferred, it
> >> keeps from having to either store data on the stack or dynamically
> >> allocate it for the copy.  But it's not a huge deal in this case.  A
> >> pointer will require you to dynamically allocate the smi_info structure
> >> so you can free it separately.  But then only the top-level put routine
> >> is required, it can simply free the structure if the refcount is zero.
> >>      
> > When the pointer mechanism is used, we will have to allocate the
> > smi_info structure dynamically. Every time the function of
> > ipmi_get_smi_info, it will be allocated dynamically. And if it fails in
> > the allocation, we can't return the expected value.
> >    
> Well, you misunderstand.  You allocate one copy when the SMI info is 
> created.  And you return a pointer to that with the refcount 
> incremented.  No need to allocate a new one on each call.  Use the 
> refcounts to know when to free it.

The ipmi_smi_info is defined statically in the structure of smi_info. 
	struct smi_info {
		......
        	struct ipmi_smi_info smi_data;
 };

If the pointer mechanism is selected, we can return the pointer of
smi_data. And in such case it seems unnecessary to know when to free it.

> 
> > But when the copy is used, it will be much simpler.  It is the caller's
> > responsibility to prepare the corresponding data structure. They can
> > define it on stack. Of course they can also dynamically allocate it.
> >
> > Can we choose the copy mechanism to make it much simpler?
> >    
> Sure, I think I already said this :).  Just get rid of the refcount stuff.

OK. I will remove the refcount stuff.

> -corey