[XS] Change in osmo-mgw[master]: mgcp_msg: Improve logging checking MGCP line param - gerrit-log

21 Jan 2025

Attention is currently required from: daniel, neels, pespin.

osmith has posted comments on this change by pespin. (
https://gerrit.osmocom.org/c/osmo-mgw/+/39352?usp=email )

Change subject: mgcp_msg: Improve logging checking MGCP line param
......................................................................

Patch Set 3:

(1 comment)

Patchset:

PS1: 
...
  this patch might be a misunderstanding from CR on the
CRCX parsing patch? 
For others reading along, Neels is referring to the discussion here:
https://gerrit.osmocom.org/c/osmo-mgw/+/39224/4..6/src/libosmo-mgcp/mgcp_ms…

...
  for me the important part is the logging context --
layering is only secondary, mentioned to serve adding logging context without layer
violations. 
I'm trying to follow, but I don't get what you mean:

Old code:
```c
if (endp)
	LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n",
line);
else
	LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, "wrong MGCP option format:
'%s'\n", line);
```

New code:
```c
LOGP(DLMGCP, LOGL_NOTICE, "%s: wrong MGCP option format: '%s'\n",
			     pdata->epname, line);
```

I've looked at what `LOGPENDP` does, and all it does is add the endpoint name:
```
#define LOGPENDP(endp, cat, level, fmt, args...) \
LOGP(cat, level, "endpoint:%s " fmt, \
     endp ? endp->name : "none", \
     ## args)
```

So... it seems the end result is the same? In any case, I don't think it is worth
blocking the bigger patch over this, I'll add my +1 there.

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39352?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I18fd316a2d830b9418a806e32f27558d980259d6
Gerrit-Change-Number: 39352
Gerrit-PatchSet: 3
Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel &lt;dwillmann(a)sysmocom.de&gt;
Gerrit-Reviewer: dexter &lt;pmaier(a)sysmocom.de&gt;
Gerrit-CC: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Gerrit-CC: osmith &lt;osmith(a)sysmocom.de&gt;
Gerrit-Attention: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: daniel &lt;dwillmann(a)sysmocom.de&gt;
Gerrit-Comment-Date: Tue, 21 Jan 2025 13:11:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt;