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