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

Re: [Libvir] [PATCH 2/6] Inactive domain support: new virHash APIs



On Wed, Nov 15, 2006 at 04:50:55AM -0500, Daniel Veillard wrote:
> On Wed, Nov 15, 2006 at 02:18:48AM +0000, Daniel P. Berrange wrote:
> > The attached patch adds a couple of new APIs to the hash table object to
> > allow various different ways of iterating over the contents of the hash
> > table. The methods are:
> > 
> >  virHashForEach
> >  virHashRemoveSet
> >  virHashSearch
> > 
> > Docs for these methods are all inline. Compared to previous patch a logic
> > flaw in the virHashRemoveSet method was fixed prevently some severe memory
> > corruption!
> 
>   The APIs are okay, I'm just wondering if the iterator should not return 
> an int allowing to break the iteration, but admitedly that would make it
> close to the search. So it's fine as-is.

Yes, that's a good idea - I'll make it return number of elements - I think
I could acutaly make use of that elsewhere already.

> > + int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, virHashDeallocator f, const void *data) {
> > +     int i;
> > + 
> > + }
> 
>  Iterating when removing entries which are first in the list is a bit tricky
> but that's looks fine.

Yeah, that's why i wrote a dedicated method for iterating & removing in
one go - calling 'virHashRemove' from the normal iterator just caused
very bad things to happen :-)

> > + void *virHashSearch(virHashTablePtr table, virHashSearcher iter, const void *data) {
> > +     int i;
> > + 
> > +     if (table == NULL || iter == NULL)
> > +         return (NULL);
> > + 
> > +     for (i = 0 ; i < table->size ; i++) {
> > +         virHashEntryPtr entry = table->table + i;
> > +         while (entry) {
> > +             if (entry->valid) {
> > +                 if (iter(entry->payload, entry->name, data))
> > +                     return entry->payload;
> > +             }
> > +             entry = entry->next;
> > +         }
> > +     }
> > +     return (NULL);
> > + }
> 
> Minor stylistic issue I tend to use return(value); and you use sometime
> return (value); or return value; but it's really not a serious problem :-)

My own style is normally 'return value', but I try to match your style
when working on libvirt - sometimes i forget though, so I'll fix up the
bits where I forget the ().

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


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