<p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21829">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21829/1/src/osmo-bsc/abis_om2000.c">File src/osmo-bsc/abis_om2000.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/osmo-bsc/+/21829/1/src/osmo-bsc/abis_om2000.c@2310">Patch Set #1, Line 2310:</a> <code style="font-family:monospace,monospace">   trx->rbs2000.trx_fi = fi;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">in an alloc() function (returning the allocated struct) I would expect this to be done by the caller […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">My original idea was to do as much as possible inside the function, to avoid having to repeat the same pattern around the function call at multiple caller sites, where the chance is high to change only one of the callers and forget about others.  I have to check if we actually do have multiple callers left in the latet iteration of the code. If not, I can go for your proposal.  Otherwise, I'd rather rename the function like _alloc_and_link or so.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21829/1/src/osmo-bsc/abis_om2000.c@2326">Patch Set #1, Line 2326:</a> <code style="font-family:monospace,monospace"> if (strcmp(osmo_fsm_inst_state_name(bts_fi), "DONE") &&</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">better check against bts->fi->state == <enum></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">yes, but that would mean the enum must be declared here already, which would mean that the enum definition no longer is next to the FSM, which I really like about this current code.  So I decided to go for the strcmp, after all it is a super infrequent operation.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21829">change 21829</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/c/osmo-bsc/+/21829"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia37cffff5c451e1d79a52ccae41ab5718b4661d4 </div>
<div style="display:none"> Gerrit-Change-Number: 21829 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 22 Dec 2020 11:01:22 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>