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

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.

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