From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: Merging ns_parse from Alpine
Date: Sun, 14 Dec 2014 17:56:38 -0500 [thread overview]
Message-ID: <20141214225638.GH4574@brightrain.aerifal.cx> (raw)
In-Reply-To: <20141214190536.GA11453@euler>
On Sun, Dec 14, 2014 at 08:05:36PM +0100, Felix Janda wrote:
> Rich Felker wrote:
> > On Sun, Dec 14, 2014 at 08:38:15AM +0100, Felix Janda wrote:
> > > Rich Felker wrote:
> > > > I'm working on merging Timo's patch for ns_parse:
> > > >
> > > > http://git.alpinelinux.org/cgit/aports/tree/main/musl/1001-add-basic-dns-record-parsing-functions.patch?id=81d50064c335467fdfd80368bac6707d70db1af7
> > > >
> > > > The first issue that came up in the process is that arpa/nameser.h,
> > > > which was previously not used by musl itself and really should never
> > > > have been accepted in its current form, is full of junk like
> > > > statement-expressions. Including it in a file that will be compiled
> > > > with musl adds build dependency on these nonstandard features. I
> > > > cleaned that up with no problem (just un-inlining the macros since
> > > > we're adding function versions anyway), but there are a few more
> > > > issues.
> > >
> > > The NS_GET* macros still seem to be used a lot in the code.
> >
> > Yes because they also advance the pointer, and this behavior was
> > intentional in the code. I don't think it hurts to use them once
> > they're fixed to be function calls.
>
> I likely just missunderstood you. With un-inlining the macros you mean
> making them call the ns_get* functions? (That seems reasonable.)
Right:
#define NS_GET16(s, cp) (void)((s) = ns_get16(((cp)+=2)-2))
#define NS_GET32(l, cp) (void)((l) = ns_get32(((cp)+=4)-4))
#define NS_PUT16(s, cp) ns_put16((s), ((cp)+=2)-2)
#define NS_PUT32(l, cp) ns_put32((l), ((cp)+=4)-4)
> > > > r = dn_expand(handle->_msg, handle->_eom, handle->_msg_ptr, rr->name, NS_MAXDNAME);
> > > > if (r < 0) return -1;
> > >
> > > dn_expand doesn't set errno.
> >
> > Maybe we should just call ns_name_uncompress (below) instead here?
> >
> > > [...]
> > > > int ns_name_uncompress(const unsigned char *msg, const unsigned char *eom,
> > > > const unsigned char *src, char *dst, size_t dstsiz)
> > > > {
> > > > int r;
> > > > r = dn_expand(msg, eom, src, dst, dstsiz);
> > > > if (r < 0) errno = EMSGSIZE;
> > > > return r;
> > > > }
> >
> > Does that sound better?
>
> That should work. I have no particular opinion on whether to call
> ns_name_uncompress or to jump to "size".
Indeed, ns_name_uncompress doesn't set any errors except EMSGSIZE. The
only ways dn_expand can fail are truncated buffers or malformed
messages that would attempt to read outside the buffer, and I think
EMSGSIZE is reasonable for either of these.
Rich
prev parent reply other threads:[~2014-12-14 22:56 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-14 0:43 Rich Felker
2014-12-14 7:38 ` Felix Janda
2014-12-14 17:23 ` Rich Felker
2014-12-14 19:05 ` Felix Janda
2014-12-14 22:56 ` Rich Felker [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141214225638.GH4574@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).