<p><a href="https://gerrit.osmocom.org/9939">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/9939/1/src/vty/command.c">File src/vty/command.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/9939/1/src/vty/command.c@1322">Patch Set #1, Line 1322:</a> <code style="font-family:monospace,monospace">          enum match_type min, bool recur)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Agree.  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I started out trying to free when needed, but hell, it is so stupidly confusing (especially with that 'matched' variable being set in different places, each needing a decision whether the pointer is a char* to free or not to free) that I decided: to have any degree of readable code, just talloc and then free all of it at the end. I think a talloc ctx is a lean little tool that fits the task pretty nicely.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Refactor: how!? the design is so broken that there is no easy way to do it. We could always copy to 'matched', but we hardly ever have optional args and it feels dumb to copy all strings all the time (for string comparison!) just because of some rare corner case. The only other thing I can think of is to very early during string parsing get rid of those stupid square brackets and flip a flag that says 'optional'. But that would be a largeish refactoring of the vty code, and I'm having enough of large refactorings right now.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I desperately need this/a fix to get on with the endless osmo-bsc inter-bsc HO, I really think this fix is good enough, and I frankly feel like bikeshed syndrome here: the large important patch has seen review only by laforge, while this easy bit here gets all sorts of tangents... :P (sheesh, have I really written 3 paragraphs on this now!?)</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/9939">change 9939</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/9939"/><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: Ia71ba742108b5ff020997bfb612ad5eb30d04fcd </div>
<div style="display:none"> Gerrit-Change-Number: 9939 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 11 Jul 2018 13:48:53 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>