<p><a href="https://gerrit.osmocom.org/12763">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12763/7/src/osysmon_main.c">File src/osysmon_main.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/12763/7/src/osysmon_main.c@252">Patch Set #7, Line 252:</a> <code style="font-family:monospace,monospace">            sleep(1);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">why not always polling osmocom event loop with a timeout of 1 second instead of instant polling + sleep?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12763/5/src/osysmon_openvpn.c">File src/osysmon_openvpn.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/12763/5/src/osysmon_openvpn.c@263">Patch Set #5, Line 263:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">What makes you think that? It should be automatically cleaned by talloc on vty refresh.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why keeping it until then if it's not longer needed? Do we believe on garbage collectors now?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12763/7/src/osysmon_openvpn.c">File src/osysmon_openvpn.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/12763/7/src/osysmon_openvpn.c@45">Patch Set #7, Line 45:</a> <code style="font-family:monospace,monospace">       fprintf(stderr, "OpenVPN [%s]: " fmt, make_authority(ctx, vpn->cfg), ##args)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">make_authority memleaks here, or at least unnecesarily delays  its free.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12763/7/src/osysmon_openvpn.c@67">Patch Set #7, Line 67:</a> <code style="font-family:monospace,monospace">      uint8_t *m = msgb_data(msg);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">would be clearer to cast here already if we know it's a string:<br>char *m = (char*) msgb_data(msg);</p><p style="white-space: pre-wrap; word-wrap: break-word;">then you can remove the cast in line 80.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12763/7/src/osysmon_openvpn.c@104">Patch Set #7, Line 104:</a> <code style="font-family:monospace,monospace">static struct openvpn_client *openvpn_client_find_or_make(const struct osysmon_state *os,</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">that's not making aything, its only finding.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/12763">change 12763</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/12763"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-sysmon </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I4493e19b9a09dcebd289457eacd1719f7f8cc31c </div>
<div style="display:none"> Gerrit-Change-Number: 12763 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 07 Feb 2019 17:29:06 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>