bug-parted
[Top][All Lists]
Advanced

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

Re: [patch] ped_device_from_store


From: Andrew Clausen
Subject: Re: [patch] ped_device_from_store
Date: Wed, 15 Aug 2001 22:57:52 +1000
User-agent: Mutt/1.2.5i

On Tue, Aug 14, 2001 at 10:57:25AM +0200, Neal H Walfield wrote:
> This patch adds a new function, ped_device_from_store, to libparted.
> This is useful only on GNU/Hurd and where an application would like to
> use libparted.  The function has the following prototype:
> 
>       PedDevice *ped_device_from_store (struct store *source,
>                                         int consume)

The "convention" is for constructors:

        namespace_object_new[_special_way_of_creating]()

So, that should be:

        ped_device_new_from_store()

> This function creates a PedDevice from SOURCE with a single reference;
> the PedDevice is not registered in parted's list of available devices.
> If CONSUME is true, SOURCE will be destoryed when the reference count of
> the PedDevice drops to zero.

I'm not convinced this is The Right Thing TM.

There are 2 options:
 * common code:

        void use_device (PedDevice* dev)
        {
                ped_device_open (dev);
                ...
                ped_device_close (dev);
        }

 * your suggestion:
 
        dev = ped_device_new_from_store (store); /* dev->open_count == 1 */
        use_device (dev);
        ped_device_close (dev);         /* device vanishes */

 * other option:

        dev = ped_device_new_from_store (store); /* dev->open_count == 0 */
        use_device (dev);
        ped_device_destroy (dev);

Your suggestion isn't really doing garbage collection... the
ped_device_close(dev) is explicitly destroying the device, since it
would normally be illegal to close a device that is already closed
(with dev->open_count == 0, which is what you normally expect from a
device constructor).

So, if you are explicitly destroying the device, then use the standard
interface, ped_device_destroy().

Obviously, there is no (public) ped_device_destroy() ATM, but that
can be fixed easily enough...

Do you want me to do this?  Or do you have objections...?

Thanks,
Andrew

PS, no _init_PedDevice, please.  _init_ped_device if you must.
(It really should be _device_init(), since it's a private method
of PedDevice)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]