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

Re: [libvirt] [PATCH 05/11] Remove pointless nwIPAddress struct & void *casts



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 :|


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