<p style="white-space: pre-wrap; word-wrap: break-word;">I agree with Vadim in that there are two separate features merged in one patch which should be separate: 1) exposing the list via CTRL, and 2) adding access modes.   I would assume '1' is the less controversial change, and hence it should be first so it can be merged independently of the second.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Whether or not we'd want to go this way with extneding the CTRL interface, and whether the proposed implementation complies with the CTRL interface in general is something I put into Daniels' hands, as he created CTRL originally.</p><p><a href="https://gerrit.osmocom.org/12814">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12814/2/include/osmocom/ctrl/control_cmd.h">File include/osmocom/ctrl/control_cmd.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12814/2/include/osmocom/ctrl/control_cmd.h@107">Patch Set #2, Line 107:</a> <code style="font-family:monospace,monospace">enum ctrl_cmd_mode mode;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You're breaking ABI here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">To give more context: As per our policy, this would require dding an entry to the TODO-RELEASE file, as it requires us to increment the LIBVERSION at the next version tag to avoid breaking backwards/forwards ABI compatibility.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12814/2/include/osmocom/ctrl/control_cmd.h@152">Patch Set #2, Line 152:</a> <code style="font-family:monospace,monospace">CTRL_CMD_DEFINE_STRUCT</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Since this is a public header, we cannot modify existing symbols nor definitions.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">this is required to ensure that old code can be built against new libraries.  So you cannot change the number/order of arguments to any existing function or macro, but must always introduce a new one while keeping the old one as a compatibility wrapper with reasonable defaults.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12814/2/src/ctrl/control_vty.c">File src/ctrl/control_vty.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12814/2/src/ctrl/control_vty.c@89">Patch Set #2, Line 89:</a> <code style="font-family:monospace,monospace">       switch(cmd_el->mode){</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Missing space, should be 'switch ('.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the philosophy of the Linux kernel coding style (which we use) is "if, switch, etc. are no functions, hence a space".</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/12814">change 12814</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/12814"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I16f59bfca72a7dd9c268bc64499b26d82a2115d2 </div>
<div style="display:none"> Gerrit-Change-Number: 12814 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Omar Ramadan <omar.ramadan93@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 06 Feb 2019 08:21:41 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>