[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] init qemu_driver's qemuImgBinary field



On 11/14/2012 01:57 AM, li guang wrote:
> 在 2012-11-14三的 02:28 -0600,Doug Goldstein写道:
>> On Wed, Nov 14, 2012 at 1:56 AM, li guang <lig fnst cn fujitsu com> wrote:
>>> 在 2012-11-14三的 01:38 -0600,Doug Goldstein写道:
>>>> On Tue, Nov 13, 2012 at 9:03 PM, liguang <lig fnst cn fujitsu com> wrote:
>>>>> Signed-off-by: liguang <lig fnst cn fujitsu com>
>>>>> ---

Your commit message needs justification why you are moving from a lazy
cache to a guaranteed initialization.

>>
>> I would argue that its bad form to do that for at least 2 reasons.
>> 1. The fact that driver->qemuImgBinary is "hidden" behind a function
>> to access it means you should effectively treat it as opaque and not
>> directly access it.
> 
> well, quite conceptually,
> but, qemu_driver is global variable,
> every member can't be hidden at all.
> e.g. driver->snapshotDir was used directly.

I don't mind accessing the member variable directly, but only on
condition that we completely delete qemuFindQemuImgBinary(), and instead
inline its initialization code into the one-time initialization.  As
long as the function call remains, then code should use the function
call.  If you are trying to avoid the caching function call, then you
should avoid it everywhere, and this patch didn't go far enough.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]