On Thu, Oct 21, 2010 at 02:02:44PM -0600, Eric Blake wrote:
> On 10/21/2010 12:17 PM, Daniel P. Berrange wrote:
> >The nwIPAddress was simply a wrapper about virSocketAddr.
> >Just use the latter directly, removing all the extra field
> >de-references from code& helper APIs for parsing/formatting.
> >
> >Also remove all the redundant casts from strong types to
> >void * and then immediately back to strong types.
> >
> >* src/conf/nwfilter_conf.h: Remove nwIPAddress
> >* src/conf/nwfilter_conf.c, src/nwfilter/nwfilter_ebiptables_driver.c:
> > Update to use virSocketAddr and remove void * casts.
> >---
> > src/conf/nwfilter_conf.c | 103
> > +++++++++--------------------
> > src/conf/nwfilter_conf.h | 9 +--
> > src/nwfilter/nwfilter_ebiptables_driver.c | 4 +-
> > 3 files changed, 34 insertions(+), 82 deletions(-)
> >
> >diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> >index 40fbf5e..6fd07d4 100644
> >--- a/src/conf/nwfilter_conf.c
> >+++ b/src/conf/nwfilter_conf.c
> >@@ -1325,26 +1325,6 @@ virNWMACAddressParser(const char *input,
> > }
> >
> >
> >-static bool
> >-virNWIPv4AddressParser(const char *input,
> >- nwIPAddressPtr output)
> >-{
> >- if (virSocketParseIpv4Addr(input,&output->addr) == -1)
> >- return 0;
> >- return 1;
>
> Good change. I'd rather see functions return true/false than 0/1 when
> labeled as bool, so I'm glad to see this go.
>
> >- *(uint8_t *)storage_ptr =
> >- (uint8_t)uint_val;
> >+ item->u.u8 = (uint8_t)uint_val;
>
> Technically, the (uint8_t) cast isn't needed, either, since in C,
> assignment auto-narrows. But I don't know if it might trigger a picky
> compiler warning and thus a -Werror failure, so it's probably okay to
> leave it in.
Yes, IIRC this one was a case where I needed a cast to avoid a
compiler warning.
>
> >
> >-typedef struct _nwIPAddress nwIPAddress;
> >-typedef nwIPAddress *nwIPAddressPtr;
> >-struct _nwIPAddress {
> >- virSocketAddr addr;
> >-};
>
> I suppose nwfilter originally wrapped this, in case it needs to add
> another member. But if that is the case, then we can probably add that
> member directly in virSocketAddr, as it would probably be useful elsewhere.
I was thinking that nwIPAddress just originally pre-dated the virSocket
struct existing, either way, I think its now redundant.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list