linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Corey Minyard <minyard@acm.org>
To: ykzhao <yakui.zhao@intel.com>
Cc: "openipmi-developer@lists.sourceforge.net"
	<openipmi-developer@lists.sourceforge.net>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"lenb@kernel.org" <lenb@kernel.org>
Subject: Re: [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device
Date: Thu, 14 Oct 2010 22:03:37 -0500	[thread overview]
Message-ID: <4CB7C489.3050900@acm.org> (raw)
In-Reply-To: <1287104855.22388.34.camel@localhost.localdomain>


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.

> 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.

-corey

  reply	other threads:[~2010-10-15  3:03 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-12  7:47 [RFC PATCH 0/4_v10] IPMI/ACPI: Install the ACPI IPMI opregion yakui.zhao
2010-10-12  7:47 ` [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device yakui.zhao
2010-10-12  7:47   ` [RFC PATCH 2/4] IPMI: Remove the redundant definition of ipmi_addr_src yakui.zhao
2010-10-12  7:47     ` [RFC PATCH 3/4] IPMI: Add the document description of ipmi_get_smi_info yakui.zhao
2010-10-12  7:47       ` [RFC PATCH 4/4] IPMI/ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller yakui.zhao
2010-10-12  7:51   ` [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device ykzhao
2010-10-14 16:53   ` Corey Minyard
2010-10-15  1:07     ` ykzhao
2010-10-15  3:03       ` Corey Minyard [this message]
2010-10-15  5:07         ` ykzhao
  -- strict thread matches above, loose matches on Subject: below --
2010-10-22  9:10 [RFC PATCH 0/4_v11] IPMI/ACPI: Install the ACPI IPMI opregion yakui.zhao
2010-10-22  9:10 ` [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device yakui.zhao
2010-10-25  1:19   ` ykzhao
2010-10-25  3:25   ` Corey Minyard
2010-10-25  7:00     ` ykzhao
2010-10-26  9:14 [RFC PATCH 0/4_v11] IPMI/ACPI: Install the ACPI IPMI opregion yakui.zhao
2010-10-26  9:14 ` [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device yakui.zhao
2010-10-27 16:13   ` Corey Minyard
2010-10-28  1:00     ` ykzhao
2010-11-02  5:33       ` ykzhao
2010-11-04  0:41         ` Corey Minyard
2010-11-15  7:52           ` ykzhao
2010-11-23  7:13             ` ykzhao
2010-11-30  0:26 [RFC PATCH 0/4_v13] IPMI/ACPI: Install the ACPI IPMI opregion yakui.zhao
2010-11-30  0:26 ` [RFC PATCH 1/4] IPMI: Add one interface to get more info of low-level IPMI device yakui.zhao
2010-11-30  0:36   ` ykzhao
2010-12-06  1:06     ` ykzhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CB7C489.3050900@acm.org \
    --to=minyard@acm.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    --cc=yakui.zhao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).