<p style="white-space: pre-wrap; word-wrap: break-word;">Thanks for the reviews! I've pushed an update to address everything, except for splitting this up in multiple patches.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Please provide separate commits with an introduction for each layer and how it will be used by other objects and which objects will it use.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I'll do that tomorrow.</p><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202">View Change</a></p><p>29 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/configure.ac">File configure.ac:</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-hlr/+/16202/9/configure.ac@182">Patch Set #9, Line 182:</a> <code style="font-family:monospace,monospace"> libosmo-mslookup.pc</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Yes. We want to make mslookup integrate-able by any client programs. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">In particular, it would make sense for OsmoMSC in combination with SMS over GSUP.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/debian/control">File debian/control:</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-hlr/+/16202/9/debian/control@75">Patch Set #9, Line 75:</a> <code style="font-family:monospace,monospace">Depends: ${shlibs:Depends},</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">(@osmith? ... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/hlr/logging.h">File include/osmocom/hlr/logging.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/osmo-hlr/+/16202/9/include/osmocom/hlr/logging.h@11">Patch Set #9, Line 11:</a> <code style="font-family:monospace,monospace"> DMSLOOKUP,</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Using this name isn't really a problem, is it? Any abbreviation I can think of would make it hard to […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Changed to DMSLKP.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mdns_sock.h">File include/osmocom/mslookup/mdns_sock.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/osmo-hlr/+/16202/9/include/osmocom/mslookup/mdns_sock.h@1">Patch Set #9, Line 1:</a> <code style="font-family:monospace,monospace">/* Copyright 2019 by sysmocom s.f.m.c. GmbH <info@sysmocom.de></code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Looks like this file and mdns.h can be merged.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">mdns_sock is just about the socket related functions, while mdns.h is part of the stack that builds the mdns packets (therefore I'd rather not merge the two files):</p><ul><li>mdns.h: encoding and decoding mslookup query/result</li><li>mdns_msg.h/mdns_record.h: encoding and decoding whole dns packets (request/answer); answer consists of multiple records</li><li>mdns_rfc.h: encoding and decoding sections of dns packets (header, question/record) and qname</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">mdns_msg.h and mdns_record.h will be merged, as discussed with Neels.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'll split this patch up, to introduce one layer at a time. Then this hopefully makes more sense, and it also makes reviewing easier.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mdns_sock.h@26">Patch Set #9, Line 26:</a> <code style="font-family:monospace,monospace"> struct addrinfo *ai;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">ah, you mean the #include</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Added #include <netdb.h>.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mdns_sock.h@30">Patch Set #9, Line 30:</a> <code style="font-family:monospace,monospace"> int (*cb)(struct osmo_fd *fd, unsigned int what),</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">the API doc in mdns_sock.c hints at it, but I'm adding a comment there.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Couldn't find your update (maybe not committed?), but I've updated the apidoc comment to:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> * \param[in] cb callback for incoming data that will be passed to osmo_fd_setup (should read from osmo_fd->fd).</pre></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mdns_sock.h@32">Patch Set #9, Line 32:</a> <code style="font-family:monospace,monospace">int osmo_mdns_sock_send(const struct osmo_mdns_sock *mdns_sock, struct msgb *msg);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">(it doesn't close the socket implicitly. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Looking at sendto being used in libosmocore, we don't implicitly close the socket. Instead we return sendto's return code. But the callers never close the socket on error from what I can find:</p><ul><li>https://git.osmocom.org/libosmocore/tree/src/gb/gprs_ns.c?id=8a7eed50dbd7fc05a1c3bbf302ef8e42a5698a98#n2012</li><li>https://git.osmocom.org/libosmocore/tree/src/gb/gprs_ns_frgre.c?id=8a7eed50dbd7fc05a1c3bbf302ef8e42a5698a98#n142</li><li>https://git.osmocom.org/libosmocore/tree/src/gb/gprs_ns_frgre.c?id=8a7eed50dbd7fc05a1c3bbf302ef8e42a5698a98#n306</li><li>https://git.osmocom.org/libosmocore/tree/src/stats.c?id=8a7eed50dbd7fc05a1c3bbf302ef8e42a5698a98#n459</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">This means, not doing it implicitly would follow what we are doing already.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mslookup.h">File include/osmocom/mslookup/mslookup.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/osmo-hlr/+/16202/9/include/osmocom/mslookup/mslookup.h@29">Patch Set #9, Line 29:</a> <code style="font-family:monospace,monospace">#define OSMO_MSLOOKUP_SERVICE_HLR_GSUP "gsup.hlr"</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">We could in fact also drop them, except for the "gsup. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done (Neels removed them)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mslookup.h@113">Patch Set #9, Line 113:</a> <code style="font-family:monospace,monospace">int osmo_mslookup_query_from_domain_str(struct osmo_mslookup_query *q, const char *domain);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">ack</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done (Neels renamed it, just replying so I can mark this as "resolved" in gerrit. Same with following "Done" comments, if Neels already replied with "ack" etc., he fixed them and I'm just marking it as resolved here so I know what is still open.)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mslookup_client.h">File include/osmocom/mslookup/mslookup_client.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/osmo-hlr/+/16202/9/include/osmocom/mslookup/mslookup_client.h@35">Patch Set #9, Line 35:</a> <code style="font-family:monospace,monospace">/*! This part of a lookup request is not seen by the individual query method implementations. */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">adjusting API doc....</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/include/osmocom/mslookup/mslookup_client.h@95">Patch Set #9, Line 95:</a> <code style="font-family:monospace,monospace">struct osmo_mslookup_client_method {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">ack</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/libosmo-mslookup.pc.in">File libosmo-mslookup.pc.in:</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-hlr/+/16202/9/libosmo-mslookup.pc.in@9">Patch Set #9, Line 9:</a> <code style="font-family:monospace,monospace">Libs: -L${libdir} @TALLOC_LIBS@ -losmogsm -losmo-mslookup -losmocore</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">hmm, checking... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c">File src/mslookup/mdns.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-hlr/+/16202/9/src/mslookup/mdns.c@1">Patch Set #9, Line 1:</a> <code style="font-family:monospace,monospace">/* mslookup specific functions for encoding and decoding mslookup queries/results into mDNS packets, using the high</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">(leaving this file for osmith)</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c@76">Patch Set #9, Line 76:</a> <code style="font-family:monospace,monospace"> msgb_free(msg);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">missing "talloc_free(req.domain);" here? […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c@86">Patch Set #9, Line 86:</a> <code style="font-family:monospace,monospace"> * NULL on error.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">No need for new line.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Removed this new line and similar ones from all the \returns apidocs added in this patch.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c@98">Patch Set #9, Line 98:</a> <code style="font-family:monospace,monospace"> query = talloc_zero(ctx, struct osmo_mslookup_query);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">should OSMO_ASSERT(query) here</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c@106">Patch Set #9, Line 106:</a> <code style="font-family:monospace,monospace"> if (req)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">this if is not needed.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c@108">Patch Set #9, Line 108:</a> <code style="font-family:monospace,monospace"> if (query)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">this if is not needed.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns.c@127">Patch Set #9, Line 127:</a> <code style="font-family:monospace,monospace"> osmo_sockaddr_str_from_32(sockaddr_str, *(uint32_t *)rec->data, 0);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">use osmo_loadbe or whatever here, you may incurr into unaligned access?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">osmo_sockaddr_str_from_32 expects the input to be network-byte-order.</p><p style="white-space: pre-wrap; word-wrap: break-word;">https://git.osmocom.org/libosmocore/tree/src/sockaddr_str.c?id=8a7eed50dbd7fc05a1c3bbf302ef8e42a5698a98#n258</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_msg.c">File src/mslookup/mdns_msg.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-hlr/+/16202/9/src/mslookup/mdns_msg.c@92">Patch Set #9, Line 92:</a> <code style="font-family:monospace,monospace"> * -EINVAL on error.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">new line not needed.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_msg.c@114">Patch Set #9, Line 114:</a> <code style="font-family:monospace,monospace"> if (osmo_mdns_rfc_record_encode(ctx, msg, &rec) != 0)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm a bit undecided about this, since all osmo_mdns_rfc_*_encode functions with a msgb argument are appending to the msgb. Should I call all of them _encode_append then?</p><p style="white-space: pre-wrap; word-wrap: break-word;">I found some existing functions in libosmocore too, which are appending to a msgb, but are also just called _encode(), not _encode_append():</p><ul><li>gsup.c: encode_pdp_info, encode_auth_info, osmo_gsup_encode_an_apdu</li><li>gsm48_ie.c: gsm48_encode_bearer_cap, gsm48_encode_cccap, gsm48_encode_called, ...</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">So with these findings, I'm leaning more towards keeping the encode() name (as it is more consistent) unless there is a good reason to change it.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'll update the apidocs of these osmo_mdns_rfc_*_encode functions to make clear, that they are appending to msgb.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">(it encodes a single record, makes sense to me)</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">What makes sense to you, what Pau proposed or how it was in the patch?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_record.c">File src/mslookup/mdns_record.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-hlr/+/16202/9/src/mslookup/mdns_record.c@35">Patch Set #9, Line 35:</a> <code style="font-family:monospace,monospace"> ret->data = (uint8_t *)talloc_asprintf(ctx, "%c%s=%s", (char)len, key, value);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, I'm sure that this is correct.</p><p style="white-space: pre-wrap; word-wrap: break-word;">https://tools.ietf.org/html/rfc1035#section-3.3.14</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">3.3.14. TXT RDATA format<br> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+<br> / TXT-DATA /<br> +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+<br>> where:</pre><p style="white-space: pre-wrap; word-wrap: break-word;">TXT-DATA One or more <character-string>s.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">and</p><p style="white-space: pre-wrap; word-wrap: break-word;">https://tools.ietf.org/html/rfc1035#section-3.3</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;"><character-string> is a single<br>length octet followed by that number of characters. <character-string><br>is treated as binary information, and can be up to 256 characters in<br>length (including the length octet).</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">(Which makes the length information redundant, because the resource record already has a defined length... not sure what they were thinking. -- https://tools.ietf.org/html/rfc1035#section-4.1.3 )</p><p style="white-space: pre-wrap; word-wrap: break-word;">I had this wrong at first, because I did not look up the definition of <character-string> and thought it was just a regular string, without the length byte in front. But it became obvious in wireshark, and there it also displays correctly after I've implemented <character-string> properly.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'll add a more descriptive comment, and a max length check.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_record.c@39">Patch Set #9, Line 39:</a> <code style="font-family:monospace,monospace"> ret->length = len + 1;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">and here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_record.c@45">Patch Set #9, Line 45:</a> <code style="font-family:monospace,monospace"> * NULL on error.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">all around: new line not needed.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_rfc.c">File src/mslookup/mdns_rfc.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-hlr/+/16202/9/src/mslookup/mdns_rfc.c@122">Patch Set #9, Line 122:</a> <code style="font-family:monospace,monospace"> osmo_store16be(buf->id, &buf->id);</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">wtf? is it really worth memcpying everything then changing tons of stuff in place?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">The struct osmo_mdns_rfc_header has bitfields and uint16:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> struct osmo_mdns_rfc_header {<br> #if OSMO_IS_LITTLE_ENDIAN<br> uint16_t id;<br> uint8_t rd:1,<br> tc:1,<br> aa:1,<br> opcode:4,<br> qr:1; /* QR (0: query, 1: response) */<br> uint8_t rcode:4,<br> z:3,<br> ra:1;<br> uint16_t qdcount; /* Number of questions */<br> uint16_t ancount; /* Number of answers */<br> uint16_t nscount; /* Number of authority records */<br> uint16_t arcount; /* Number of additional records */<br> #elif OSMO_IS_BIG_ENDIAN<br> ...<br> #endif<br> } __attribute__ ((packed));</pre><p style="white-space: pre-wrap; word-wrap: break-word;">I'm doing the memcpy to fill out the bitfields correctly, and osmo_store16be to fix up the uint16_ts.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">This should suffice:</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">This does not work, because then the bytes in uint16_t (id, qdcount, ...) are swapped.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Wireshark shows 256 questions (question sections) instead of one.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> diff --git a/tests/mslookup/mdns_test.err b/tests/mslookup/mdns_test.err<br> index 51e5afe6..3cb1ddcb 100644<br> --- a/tests/mslookup/mdns_test.err<br> +++ b/tests/mslookup/mdns_test.err<br> @@ -95,7 +95,7 @@ header in:<br> .ancount = 0<br> .nscount = 0<br> .arcount = 0<br> -encoded: 05 39 00 00 00 01 00 00 00 00 00 00 <br> +encoded: 39 05 00 00 01 00 00 00 00 00 00 00 <br> header out:<br> .id = 1337<br> .qr = 0<br> @@ -128,7 +128,7 @@ header in:<br> .ancount = 1111<br> .nscount = 2222<br> .arcount = 3333<br> -encoded: 00 2a 97 a3 04 d2 04 57 08 ae 0d 05 <br> +encoded: 2a 00 97 a3 d2 04 57 04 ae 08 05 0d <br> header out:<br> .id = 42<br> .qr = 1</pre></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_rfc.c@134">Patch Set #9, Line 134:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">here just […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Does not work, see above.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_sock.c">File src/mslookup/mdns_sock.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-hlr/+/16202/9/src/mslookup/mdns_sock.c@40">Patch Set #9, Line 40:</a> <code style="font-family:monospace,monospace"> * will not only be called when someone else is sending data, but also for data that was sent from this osmo_mdns_sock.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">the point is that a multicast socket receives everything, also what it sends itself, so as soon as y […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mdns_sock.c@44">Patch Set #9, Line 44:</a> <code style="font-family:monospace,monospace"> * \param[in] cb callback for incoming data (should read from osmo_fd->fd).</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I have this now, which should make it obvious that it becomes "struct osmo_fd"'s callback:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> * \param[in] cb callback for incoming data that will be passed to osmo_fd_setup (should read from osmo_fd->fd).</pre></blockquote></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202/9/src/mslookup/mslookup.c">File src/mslookup/mslookup.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-hlr/+/16202/9/src/mslookup/mslookup.c@148">Patch Set #9, Line 148:</a> <code style="font-family:monospace,monospace"> if (result->host_v4.ip[0]) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">host_v4.ip is a char[], it is checking for an empty string.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16202">change 16202</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-hlr/+/16202"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-hlr </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I83487ab8aad1611eb02e997dafbcb8344da13df1 </div>
<div style="display:none"> Gerrit-Change-Number: 16202 </div>
<div style="display:none"> Gerrit-PatchSet: 10 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 26 Nov 2019 16:03:28 +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"> Comment-In-Reply-To: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>