[PATCH 3/3] gtp: Handle gtpv1 in gtp_update_pdp_conf() correctly

[Harald Welte]

Hi Daniel,

On Wed, Feb 03, 2016 at 06:53:31PM +0100, Daniel Willmann wrote:
> +			if (gtpie_gettv0(ie, GTPIE_QOS_PROFILE0, 0,
> +					 &pdp->qos_neg0,
> +					 sizeof(pdp->qos_neg0))) {
> +				gsn->missing++;
> +				GTP_LOGPKG(LOGL_ERROR, peer,
> +					    pack, len,
> +					    "Missing conditional information field\n");
> +				if (gsn->cb_conf)
> +					gsn->cb_conf(type, EOF, pdp, cbp);
> +				/*    if (gsn->cb_delete_context) gsn->cb_delete_context(pdp);
> +				   pdp_freepdp(pdp); */
> +				return EOF;
> +			}
> +
> +			if (gtpie_gettv2(ie, GTPIE_FL_DI, 0, &pdp->flru)) {
> +				gsn->missing++;
> +				GTP_LOGPKG(LOGL_ERROR, peer,
> +					    pack, len,
> +					    "Missing conditional information field\n");
> +				if (gsn->cb_conf)
> +					gsn->cb_conf(type, EOF, pdp, cbp);
> +				/*    if (gsn->cb_delete_context) gsn->cb_delete_context(pdp);
> +				   pdp_freepdp(pdp); */
> +				return EOF;
> +			}

This looks like a bit too much of copy+paste to me.  Can't we put that
into a separate function, or if not, at least a macro that hides all the
verbosity and the copy+paste?  Also, why is ther a commented-out section
about cb_delete_context involved?


-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)