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

Re: [libvirt] [PATCH V4 1/3] Add a class for file descriptor sets



On Thu, Feb 07, 2013 at 10:35:14PM -0500, Stefan Berger wrote:
> Rather than passing the next-to-use file descriptor set Id
> and the hash table for rembering the mappings of aliases to
> file descriptor sets around, encapsulate the two in a class.
> 
> Signed-off-by: Stefan Berger <stefanb linux vnet ibm com>

Nit-picking, I'd rename  "Fdset" to "FdSet" since we usually
capitalize each new word, and "Fd" and "Set" are really separate
words here.

> Index: libvirt/src/util/virfdset.h
> ===================================================================
> --- /dev/null
> +++ libvirt/src/util/virfdset.h
> +#ifndef __FDSET_H__
> +# define __FDSET_H__
> +
> +# include "internal.h"
> +# include "virbuffer.h"
> +# include "virxml.h"
> +# include "virhash.h"
> +
> +typedef struct _virFdset virFdset;
> +typedef virFdset *virFdsetPtr;
> +
> +struct _virFdset {
> +    virHashTablePtr aliasToFdset;
> +    unsigned int nextfdset;
> +};

It'd be preferrable for the struct to be in the .c file to make
the class representation completely opaque to callers.

I'd also suggest making it a virObject

> +/**
> + * virFdsetInit
> + * @fdset: fdset
> + *
> + * Initialize the @fdset.
> + * Returns 0 on success, -1 on failure.
> + */
> +int virFdsetInit(virFdsetPtr fdset);

I'd prefer this to use our more normal paradigm of

   virFdsetPtr virFdsetNew(void);



> +
> +/**
> + * virFdsetReset
> + * @fdset: fdset
> + *
> + * Reset the @fdset and forget about all mappings
> + * of aliases to file descriptor set data
> + */
> +void virFdsetReset(virFdsetPtr fdset);
> +
> +/**
> + * virFdsetClear
> + * @fdset: the fdset
> + *
> + * Free all memory associated with the @fdset but do not free
> + * the fdset structure itself. This is the counter part to
> + * virFdsetInit.
> + */
> +void virFdsetClear(virFdsetPtr fdset);

And just rely on virObjectUnref()

> +
> +
> +/**
> + * virFdsetRemoveAlias
> + * @fdset: the fdset
> + * @alias: the alias to remove
> + *
> + * Remove the given alias' mapping from @fdset
> + */
> +void virFdsetRemoveAlias(virFdsetPtr fdset, const char *alias);

  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

> +
> +
> +/**
> + * virFdsetNextSet
> + * @fdset: fdset
> + * @alias: device alias
> + * @fdset: pointer to unsigned int for storing the file descriptor set id
> + *
> + * Get the next file descriptor set number and store it with the given
> + * @alias. If successful, return the file descriptor set id in @fdsetnum.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +int virFdsetNextSet(virFdsetPtr fdset, const char *alias,
> +                    unsigned int *fdsetnum);

  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);


> +/**
> + * virFdsetFormatXML
> + * @fdset: fdset
> + * @buf: virBufferPtr for writing into
> + *
> + * Write XML representation of the @fdset into the buffer @buf.
> + */
> +void virFdsetFormatXML(virFdsetPtr fdset, virBufferPtr buf);

  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

> +
> +/**
> + * virFdsetParseXML
> + * @fdset: fdset
> + * @xPath: xpath expression to find the @fdset's XML nodes
> + * @ctxt: xpath context
> + *
> + * Parse the fdset XML representation and collect the data into @fdset.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> +int virFdsetParseXML(virFdsetPtr fdset, const char *xPath,
> +                     xmlXPathContextPtr ctxt);

  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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