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_msg.c#b257

> 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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-CC: neels <[email protected]>
Gerrit-CC: osmith <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Attention: daniel <[email protected]>
Gerrit-Comment-Date: Tue, 21 Jan 2025 13:11:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <[email protected]>
Comment-In-Reply-To: pespin <[email protected]>

Reply via email to