On 01/18/2012 09:20 AM, Stefan Berger wrote:
> Compare two filter definitions for equality and only rebuild/instantiate
> the new filter if the two filters are found to be different. This improves
> performance during an update of a filter with no obvious change or the reloading
> of filters during a 'kill -SIGHUP'
>
> Unforuntately this more involved than a mere memcmp() on the structures.
s/Unforuntately/Unfortunately/
>
> ---
> src/Makefile.am | 1
> src/conf/nwfilter_conf.c | 213 ++++++++++++++++++++
> src/conf/nwfilter_params.c | 18 +
> src/conf/nwfilter_params.h | 2
> src/conf/nwfilter_protocols.c | 447 ++++++++++++++++++++++++++++++++++++++++++
> src/conf/nwfilter_protocols.h | 56 +++++
> 6 files changed, 737 insertions(+)
Fails 'make syntax-check':
prohibit_empty_lines_at_EOF
src/conf/nwfilter_protocols.c
maint.mk: empty line(s) or no newline at EOF
And while fixing that, I noticed that you didn't bump any copyright
years. Emacs users can get this for free with this in ~/.emacs (not
sure if vim has a comparable hook):
(require 'copyright)
(defun my-copyright-update (&optional arg)
"My improvements to `copyright-update'."
(interactive "*P")
(and (not (eq major-mode 'fundamental-mode))
(copyright-update arg))
nil)
(add-hook 'before-save-hook 'my-copyright-update)
> Index: libvirt-iterator/src/conf/nwfilter_params.c
> ===================================================================
> --- libvirt-iterator.orig/src/conf/nwfilter_params.c
> +++ libvirt-iterator/src/conf/nwfilter_params.c
> @@ -747,6 +747,19 @@ err_exit:
> return -1;
> }
>
> +bool
> +virNWFilterHashTableEqual(const virNWFilterHashTablePtr hash1,
> + const virNWFilterHashTablePtr hash2)
> +{
> + if (hash1 == hash2)
> + return true;
> +
> + if (!hash1 || !hash2)
> + return false;
> +
> + return virHashEqual(hash1->hashTable, hash2->hashTable,
> + (virHashValueComparator)strcmp);
Indentation - I'd remove 3 spaces, so that the case lines up with hash1
on the line before.
> Index: libvirt-iterator/src/conf/nwfilter_protocols.c
> ===================================================================
> --- /dev/null
> +++ libvirt-iterator/src/conf/nwfilter_protocols.c
> @@ -0,0 +1,447 @@
> +/*
> + * nwfilter_protocols.c: protocol handling
> + *
> + * Copyright (C) 2011 IBM Corporation
It's 2012, now :)
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
> +
> +#define NWITEMDESCEQUAL(ITEM) \
> + virNWFilterNWItemDescEqual(&def1->ITEM, &def2->ITEM)
You don't have to change if you don't want, but it took me a while to
parse that name. Maybe NW_ITEM_DESC_EQUAL would be more readable.
> Index: libvirt-iterator/src/conf/nwfilter_protocols.h
> ===================================================================
> --- /dev/null
> +++ libvirt-iterator/src/conf/nwfilter_protocols.h
> @@ -0,0 +1,56 @@
> +/*
> + * nwfilter_protocols.h: protocol handling
> + *
> + * Copyright (C) 2011 IBM Corporation
2012
Big, but looks like a rather mechanical conversion of the various
protocols into a highly repetitive algorithm for comparisons. I didn't
notice any major problems, so:
ACK with nits fixed.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list