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

View Change

1 comment:

• Patchset:

  • Patch Set #1:

    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 change 39352. To unsubscribe, or for help writing mail filters, visit settings.