<blockquote>
<p>related to error cases, what would happen if you e.g. set in the IPv6 case 0x21 the dst_ip->af = AF_INET6, but later one run unto an error in the parsing. Then you would return an error to the callee, and would go to the read_ip_info path. Here the AF_INET6 would be still be set, or I am wrong?</p>
</blockquote>
<p>I believe you might be misreading the code here. I've added a comment to clarify things.</p>
<p>There is only a single error case after the <code>0x21</code> case, and that's the inability to read from the socket. In this case, we return <code>-1</code>, which in the calling function would result in a <code>goto error;</code>, which terminates the connection. We only set the <code>*_ip->af</code> after we've done most of the v2 parsing. Same thing is true for the v1 parsing: once we get to the <code>TCP{4,6}</code> part of the line, only <code>0</code> or <code>-1</code> are possible outputs for the function.</p>
<p>Your comment did help me discover that I was missing the <code>LOCAL</code> case (line 1075), and mistakenly returned <code>0</code> in that case instead of <code>1</code>, so thanks for that.</p>
<blockquote>
<p>You work on a local pointer src_ip and dst_ip in the parsing part, but directly on the c->rcv.src_port/c->rcv.dst_port - is there a particular reasons for it?</p>
</blockquote>
<p>Convenience. There is only one pointer access for <code>c->rcv.src_port</code> and <code>c->rcv.dst_port</code>, whereas there's two pointer accesses in <code>c->rcv.src_ip->...</code>, and multiple fields are accessed in <code>{src,dst}_ip</code> (<code>af</code>, <code>len</code>, <code>u</code>, etc). I'm happy to use a local pointer if requested.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/kamailio/kamailio/pull/1765#issuecomment-448167746">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AF36ZS4PgE6XhP3MBfq_wlNFljD2ZawIks5u6L9ggaJpZM4ZR-BH">mute the thread</a>.<img src="https://github.com/notifications/beacon/AF36ZRQuVGr3BQhWHw2Rr9OWrhMthABdks5u6L9ggaJpZM4ZR-BH.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@teotwaki in #1765: \u003e related to error cases, what would happen if you e.g. set in the IPv6 case 0x21 the dst_ip-\u003eaf = AF_INET6, but later one run unto an error in the parsing. Then you would return an error to the callee, and would go to the read_ip_info path. Here the AF_INET6 would be still be set, or I am wrong?\r\n\r\nI believe you might be misreading the code here. I've added a comment to clarify things.\r\n\r\nThere is only a single error case after the `0x21` case, and that's the inability to read from the socket. In this case, we return `-1`, which in the calling function would result in a `goto error;`, which terminates the connection. We only set the `*_ip-\u003eaf` after we've done most of the v2 parsing. Same thing is true for the v1 parsing: once we get to the `TCP{4,6}` part of the line, only `0` or `-1` are possible outputs for the function.\r\n\r\nYour comment did help me discover that I was missing the `LOCAL` case (line 1075), and mistakenly returned `0` in that case instead of `1`, so thanks for that.\r\n\r\n\u003e You work on a local pointer src_ip and dst_ip in the parsing part, but directly on the c-\u003ercv.src_port/c-\u003ercv.dst_port - is there a particular reasons for it?\r\n\r\nConvenience. There is only one pointer access for `c-\u003ercv.src_port` and `c-\u003ercv.dst_port`, whereas there's two pointer accesses in `c-\u003ercv.src_ip-\u003e...`, and multiple fields are accessed in `{src,dst}_ip` (`af`, `len`, `u`, etc). I'm happy to use a local pointer if requested."}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1765#issuecomment-448167746"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/1765#issuecomment-448167746",
"url": "https://github.com/kamailio/kamailio/pull/1765#issuecomment-448167746",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>