Attention is currently required from: laforge, neels.
3 comments:
File src/libosmo-mgcp/mgcp_msg.c:
Patch Set #4, Line 257: LOGP(DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line);
context-less logging is so 2009 […]
An error like this should be catched up by caller layers holding conn information, and then print context there. Otherwise maintaining all this code is a nightmare.
Who is actually calling this? who we want to log? a endp? a conn? a trunk? a bird? a spacecraft? This API is really to low level to actually have to care about that.
File src/libosmo-mgcp/mgcp_protocol.c:
Patch Set #4, Line 710: if (osmux_init(trunk) < 0) {
drops logging context: endp reduced to just trunk. […]
if this is called due to whatever endp/conn triggerer and this function fails, then it's up to the caller to log whatever context it has upon osmux initialization failure.
Again, there's too many contexts which may be of interest here depending on the code path, so let's keep it at the caller wherever it is interesting.
Patch Set #4, Line 899: endp->x_osmo_ign |= hpars->x_osmo_ign;
are you sure about `|=` instead of `=`??? […]
Yes |= because it's in the endp, it's shared by several conns. Otherwise a 2nd conn entering would remove prior information there.
If you look at the previous code you'll see it also does |=. Whether it is clean up on release or similar I don't know, but I'm not modifying the logic here.
This x_osmo_sign will probably need more cleanup in the future, but I cannot do it here, I simply stick to what we had.
To view, visit change 39224. To unsubscribe, or for help writing mail filters, visit settings.