Re: [PATCH] ipvsadm: Fix svc->pe_name conditional

["Ryan O'Hara" <rohara@redhat.com>]

On Tue, Aug 20, 2013 at 09:35:49AM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 19 Aug 2013, Ryan O'Hara wrote:
> 
> > On Mon, Aug 19, 2013 at 06:27:21PM +0300, Julian Anastasov wrote:
> > > 
> > > 	May be they can use 'goto' instead of 'return':
> > > 
> > > 	if (condition) {
> > > 		errno = XXX;
> > > 		goto out_err;
> > > 	}
> > > 
> > > 	and funcs can have such exit point:
> > > 
> > > out_err:
> > > 	free stuff on error
> > > 	return ret;
> > 
> > OK. It looks like ipvs_get_service() is the only function with a
> > potential leak. The other functions that call any of the CHECK_*
> > macros will just get an 'out_err: return -1'.
> 
> 	Yes
> 
> > I also noticed that CHECK_COMPAT_DEST can be removed. This macro
> > simply calls CHECK_IPV4, which is also called by CHECK_COMPAT_SVC. I
> > noticed that CHECK_COMPAT_DEST is only called after CHECK_COMPAT_SVC,
> > so this macro can be removed entirely. Agree?
> 
> 	No. It looks like the macros work with more
> than one structure: svc and dest. Same was for CHECK_PE
> where both structs were supported: ipvs_service_entry_t and
> ipvs_service_t when CHECK_PE was called from CHECK_COMPAT_SVC.
> 
> 	CHECK_COMPAT_SVC does not check the dest->af,
> it checks svc->af. Without both checks, svc can come
> with address from AF_INET family while -r option has address
> from AF_INET6 family. IMHO, we need just the return->goto
> change.

You're right. Not sure how I missed that.

Ryan