4 Feb
2016
4 Feb
'16
3:35 p.m.
Hi Harald,
On Wed, 2016-02-03 at 19:12 +0100, Harald Welte wrote:
On Wed, Feb 03, 2016 at 06:53:31PM +0100, Daniel
Willmann wrote:
true. Unfortunately, the whole code looks like this and I tried to stick
with the "style".
+ 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?
A function or macro will need 7 variables passed to it which makes the
whole thing quite ugly (unless the macro makes assumptions about the
local variable names). I have now put the error handling code at the end
of the function and jump to it in case of an error.
We lose the ability to infer the missing element from the line number of
the log message, but I don't think it's a big issue. The message is
still logged and wireshark can be used for debugging as well.
The section about cb_delete_context was there before (and still is in
other parts of the code). I have removed it in that function now.
Regards
Daniel
--
- Daniel Willmann <dwillmann(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte