Messages in this thread | | | Date | Wed, 03 Feb 2016 22:07:03 +0100 | From | Daniel Borkmann <> | Subject | Re: [PATCH] vsprintf: do not append unset Scope ID to IPv6 |
| |
On 02/03/2016 11:41 AM, Jason A. Donenfeld wrote: > The sockaddr_in6 has the sin6_scope_id parameter. This contains the > netdev index of the output device. When set to 0, sin6_scope_id is > considered to be "unset" -- it has no Scope ID (see RFC4007). When it is > set to >0, it has a Scope ID. [...] > So, here, either it has Scope ID, in which case it's returned, or it > doesn't need a Scope ID, in which case 0 - "no scope id" - is returned. > > Therefore, vsprintf should treat the 0 Scope ID the same way the rest of > the v6 stack and the RFC treats it: 0 Scope ID means no Scope ID. And if > there is no Scope ID, there is nothing to be printed.
I don't see an "issue" here. I.e. if the developer requested to append 's' to the specifier, then it's expected to dump what is in the scope id, even if that is 0 meaning "unset" scope.
It would be principle of least surprise -- I could imagine if not dumped, then a developer would check the source code to find out whether he did a mistake or it's expected behavior, just to make sure. That's what I would do probably.
Is this causing a _real_ issue somewhere (I couldn't parse one out of your commit message other than 'it would be nice to have')?
> So, this patch only prints the Scope ID when one exists, and does not > print a Scope ID when one does not exist. > > It turns out, too, that this is what developers want. The most common > purpose of %pISpfsc is to show "what is in that sockaddr so that I can > have some idea of how to connect to it?" > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > lib/vsprintf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 48ff9c3..1b1b1c8 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -1194,7 +1194,7 @@ char *ip6_addr_string_sa(char *buf, char *end, const struct sockaddr_in6 *sa, > p = number(p, pend, ntohl(sa->sin6_flowinfo & > IPV6_FLOWINFO_MASK), spec); > } > - if (have_s) { > + if (have_s && sa->sin6_scope_id) { > *p++ = '%'; > p = number(p, pend, sa->sin6_scope_id, spec); > } >
| |