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
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|