<p><a href="https://gerrit.osmocom.org/13123">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/13123/4/include/osmocom/core/ip_port.h">File include/osmocom/core/ip_port.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/13123/4/include/osmocom/core/ip_port.h@47">Patch Set #4, Line 47:</a> <code style="font-family:monospace,monospace">osmo_ip_port</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">- sockaddr stores the port in network byte order, each log line would have to remember to call ntohs […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">my point was not that people should use sockaddr_storage directly, but that instead of this proposed patch, one could probably simply have a set of utility functions that work on sockaddr_storage instead of a custom-invented struct.  The string-to-binary and binary-to-string conversions could then be hidden by that API.</p><p style="white-space: pre-wrap; word-wrap: break-word;">There is no compilation problem with "ARM" per se.  There might be problems if you're building something intended for a Linux/Posix-style operating system on a "bare iron" embedded system, such as the builds we do for the osmocombb firmware (and possibly more firmware projects in the near future).  As those sysdtems have no IP adresses to begin with, the safe chocie is to simply not build anything related to networking on them.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'd like to see other people's comments on this.  I personally don't think it looks like something that I'd want to see as part of the standard libosmocore infrastructure. At least not unless I see a strong reason against my "have same API but use sockaddr_storage + conversion underneath" proposal.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13123/4/include/osmocom/core/ip_port.h@80">Patch Set #4, Line 80:</a> <code style="font-family:monospace,monospace">osmo_ip_port_to_32</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">normal convention in osmocom is lke memcpy(): Output argument is first, followed by input argument[s […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">this comment holds true whether or not we use sockaddr_storage, and whether or not we merge it to libosmocore or osmo-mgw.git</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13123">change 13123</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/13123"/><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: Id617265337f09dfb6ddfe111ef5e578cd3dc9f63 </div>
<div style="display:none"> Gerrit-Change-Number: 13123 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@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: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 08 Mar 2019 11:20:18 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>