Patch Set 2: Code-Review-1

(1 comment)

I agree that human readable prose are not really fit for the CTRL response, but 
about parsing ease: it is easy to split by '\n' and ignore the first line. With 
this patch (we separate the value part and then) split the value by ';' to get 
the same result AFAICT; so there is no really profound improvement that 
justifies breaking compat, is there?

-1: found a talloc problem...

https://gerrit.osmocom.org/#/c/5023/2/src/ctrl/control_if.c
File src/ctrl/control_if.c:

Line 506:               cmd->reply = talloc_asprintf_append(cmd->reply, "%s 
%"PRIu64";",
-1: before, this code did a talloc_asprintf() to bootstrap the cmd->reply from 
the cmd talloc context, now you start with appending to a NULL pointer. 
Instead, best use the ctrl_cmd_reply_printf() that you made me add to 
libosmocore ;)


-- 
To view, visit https://gerrit.osmocom.org/5023
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a24cc307450efdcd28168fffe477320c59fcd36
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-HasComments: Yes

Reply via email to