Attention is currently required from: jolly.

Patch set 4:Code-Review -1

View Change

4 comments:

• File src/osmo-bsc/vgcs_fsm.c:

  • Patch Set #4, Line 72: sprintf(string, "%08u", callref);

    as I wrote elsewhere, I think it's good practice to use snprintf, even if (after review) we can assume 16 bytes size is large enough for 8 decimal digits plus NUL byte.

  • Patch Set #4, Line 451: vgcs_call_fsm

    I believe we had the discussion already at some other location: Please don't use the _fsm suffix in the name of the FSM. It is known from context that it is a FSM name.

  • Patch Set #4, Line 1044: vgcs_chan_fsm

    no _fsm suffix, please

  • Patch Set #4, Line 1093: LOGP(DASCI, LOGL_ERROR, "Unable to decode Channel Type.\n");

    there are a lot of "raw" LOGP statements here, not providing any context. If you see this in the log, you will have no idea on whihc SCCP connection this was happening, related to which subscriber, which MSC, etc. only the first one has LOGPFSML() providing some context.

    Please always keep in mind that any real world use case will have more than one subscriber, so log messages must always carry some kind of context unless none is available at all.

To view, visit change 33611. To unsubscribe, or for help writing mail filters, visit settings.