Re: [patch] ped_device_from_store

[Neal H Walfield]

> > 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]()

If this is documented some where, could you point me to the location.

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

My rational is that the resource is already open and, as of right now,
the resource is destoryed when the open_count goes to 0.  Since the
resource is consumable, the other option will destroy it even before it
leaves ped_device_new_from_store.

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

I assume that if you are going to make ped_device_destory available as a
complement to ped_device_new*, you will also change the behavior of
ped_device_open/ped_device_close to not destory the resource when the
open count drops to 0.  If so, then I fail to see the continued utility
of ped_device_open/ped_device_close.  Actually, as far as I can tell,
right now, they really are not open and close meathods, but rather, a
reference counting system that you use to safely free nonvolatile
resources (e.g. you can get another file descriptor since you save the
path to the block device).

pgp0m3jS0pYzr.pgp
Description: PGP signature