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@xxxxxxxxxxxxxxxxxx>
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list