| Message ID | 875zu4d7x0.fsf@oldenburg2.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available | expand |
On 31/01/2019 16:00, Florian Weimer wrote: > 2019-01-31 Florian Weimer <fweimer@redhat.com> > > * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. > > diff --git a/nss/getent.c b/nss/getent.c > index f25de8f1fc..1159ba2636 100644 > --- a/nss/getent.c > +++ b/nss/getent.c > @@ -393,14 +393,24 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) > sockstr = sockbuf; > } > > + char scope_buf[3 * sizeof (unsigned long long int) + 2]; > + struct sockaddr_in6 *addr6 > + = (struct sockaddr_in6 *) runp->ai_addr; > + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) > + /* No scope ID present. */ > + scope_buf[0] = '\0'; > + else > + snprintf (scope_buf, sizeof (scope_buf), "%%%llu", > + (unsigned long long int) addr6->sin6_scope_id); sin6_scope_id is defines as uint32_t regardless, why not use PRIu32? > + > char buf[INET6_ADDRSTRLEN]; > - printf ("%-15s %-6s %s\n", > + printf ("%-15s%s %-6s %s\n", > inet_ntop (runp->ai_family, > runp->ai_family == AF_INET > ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, > + : &addr6->sin6_addr, > buf, sizeof (buf)), > - sockstr, > + scope_buf, sockstr, > runp->ai_canonname ?: ""); > > runp = runp->ai_next; >
* Adhemerval Zanella: > On 31/01/2019 16:00, Florian Weimer wrote: >> 2019-01-31 Florian Weimer <fweimer@redhat.com> >> >> * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. >> >> diff --git a/nss/getent.c b/nss/getent.c >> index f25de8f1fc..1159ba2636 100644 >> --- a/nss/getent.c >> +++ b/nss/getent.c >> @@ -393,14 +393,24 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) >> sockstr = sockbuf; >> } >> >> + char scope_buf[3 * sizeof (unsigned long long int) + 2]; >> + struct sockaddr_in6 *addr6 >> + = (struct sockaddr_in6 *) runp->ai_addr; >> + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) >> + /* No scope ID present. */ >> + scope_buf[0] = '\0'; >> + else >> + snprintf (scope_buf, sizeof (scope_buf), "%%%llu", >> + (unsigned long long int) addr6->sin6_scope_id); > > sin6_scope_id is defines as uint32_t regardless, why not use PRIu32? Fair enough. I also had to change the way the padding is produced. Thanks, Florian nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available 2019-02-01 Florian Weimer <fweimer@redhat.com> * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. diff --git a/nss/getent.c b/nss/getent.c index f25de8f1fc..892748656b 100644 --- a/nss/getent.c +++ b/nss/getent.c @@ -40,6 +40,7 @@ #include <netinet/in.h> #include <sys/socket.h> #include <scratch_buffer.h> +#include <inttypes.h> /* Get libc version number. */ #include <version.h> @@ -393,15 +394,30 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) sockstr = sockbuf; } + char scope[3 * sizeof (uint32_t) + 2]; + struct sockaddr_in6 *addr6 + = (struct sockaddr_in6 *) runp->ai_addr; + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) + /* No scope ID present. */ + scope[0] = '\0'; + else + snprintf (scope, sizeof (scope), "%%%" PRIu32, + addr6->sin6_scope_id); + char buf[INET6_ADDRSTRLEN]; - printf ("%-15s %-6s %s\n", - inet_ntop (runp->ai_family, - runp->ai_family == AF_INET - ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, - buf, sizeof (buf)), - sockstr, - runp->ai_canonname ?: ""); + if (inet_ntop (runp->ai_family, + runp->ai_family == AF_INET + ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr + : &addr6->sin6_addr, + buf, sizeof (buf)) == NULL) + strcpy (buf, "<invalid>"); + + int pad = 15 - strlen (buf) - strlen (scope); + if (pad < 0) + pad = 0; + + printf ("%s%-*s %-6s %s\n", + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); runp = runp->ai_next; }
On 01/02/2019 14:38, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 31/01/2019 16:00, Florian Weimer wrote: >>> 2019-01-31 Florian Weimer <fweimer@redhat.com> >>> >>> * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. >>> >>> diff --git a/nss/getent.c b/nss/getent.c >>> index f25de8f1fc..1159ba2636 100644 >>> --- a/nss/getent.c >>> +++ b/nss/getent.c >>> @@ -393,14 +393,24 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) >>> sockstr = sockbuf; >>> } >>> >>> + char scope_buf[3 * sizeof (unsigned long long int) + 2]; >>> + struct sockaddr_in6 *addr6 >>> + = (struct sockaddr_in6 *) runp->ai_addr; >>> + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) >>> + /* No scope ID present. */ >>> + scope_buf[0] = '\0'; >>> + else >>> + snprintf (scope_buf, sizeof (scope_buf), "%%%llu", >>> + (unsigned long long int) addr6->sin6_scope_id); >> >> sin6_scope_id is defines as uint32_t regardless, why not use PRIu32? > > Fair enough. I also had to change the way the padding is produced. > > Thanks, > Florian > > nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available > > 2019-02-01 Florian Weimer <fweimer@redhat.com> > > * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. > > diff --git a/nss/getent.c b/nss/getent.c > index f25de8f1fc..892748656b 100644 > --- a/nss/getent.c > +++ b/nss/getent.c > @@ -40,6 +40,7 @@ > #include <netinet/in.h> > #include <sys/socket.h> > #include <scratch_buffer.h> > +#include <inttypes.h> > > /* Get libc version number. */ > #include <version.h> > @@ -393,15 +394,30 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) > sockstr = sockbuf; > } > > + char scope[3 * sizeof (uint32_t) + 2]; I would prefer something more explicit as: /* It requires to hold a uint32_t plus a '%'. */ cat scope[((CHAR_BIT * sizeof(uint32_t) / 3) + 3) + 1]; But I don't have a strong opinion here. > + struct sockaddr_in6 *addr6 > + = (struct sockaddr_in6 *) runp->ai_addr; > + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) > + /* No scope ID present. */ > + scope[0] = '\0'; > + else > + snprintf (scope, sizeof (scope), "%%%" PRIu32, > + addr6->sin6_scope_id); > + > char buf[INET6_ADDRSTRLEN]; > - printf ("%-15s %-6s %s\n", > - inet_ntop (runp->ai_family, > - runp->ai_family == AF_INET > - ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, > - buf, sizeof (buf)), > - sockstr, > - runp->ai_canonname ?: ""); > + if (inet_ntop (runp->ai_family, > + runp->ai_family == AF_INET > + ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > + : &addr6->sin6_addr, > + buf, sizeof (buf)) == NULL) > + strcpy (buf, "<invalid>"); > + > + int pad = 15 - strlen (buf) - strlen (scope); > + if (pad < 0) > + pad = 0; Not fun of the magic constant, but it came from original code anyway. > + > + printf ("%s%-*s %-6s %s\n", > + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); Do we need to print the scope for invalid address as well? > > runp = runp->ai_next; > } >
* Adhemerval Zanella: >> @@ -393,15 +394,30 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) >> sockstr = sockbuf; >> } >> >> + char scope[3 * sizeof (uint32_t) + 2]; > > I would prefer something more explicit as: > > /* It requires to hold a uint32_t plus a '%'. */ > cat scope[((CHAR_BIT * sizeof(uint32_t) / 3) + 3) + 1]; > > But I don't have a strong opinion here. “3 digits per byte” is sufficiently universal that I find your longer version not particularly illuminating. 8-/ I have added a commented about the %. New patch below. >> + int pad = 15 - strlen (buf) - strlen (scope); >> + if (pad < 0) >> + pad = 0; > > Not fun of the magic constant, but it came from original code anyway. Right. >> + >> + printf ("%s%-*s %-6s %s\n", >> + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); > > Do we need to print the scope for invalid address as well? No, I think the invalid address can only happen (theoretically) for an unexpected address family from getaddrinfo. Thanks, Florian nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available 2019-02-06 Florian Weimer <fweimer@redhat.com> * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. diff --git a/nss/getent.c b/nss/getent.c index f25de8f1fc..df73d8fb29 100644 --- a/nss/getent.c +++ b/nss/getent.c @@ -40,6 +40,7 @@ #include <netinet/in.h> #include <sys/socket.h> #include <scratch_buffer.h> +#include <inttypes.h> /* Get libc version number. */ #include <version.h> @@ -393,15 +394,31 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) sockstr = sockbuf; } + /* Three digits per byte, plus '%' and null terminator. */ + char scope[3 * sizeof (uint32_t) + 2]; + struct sockaddr_in6 *addr6 + = (struct sockaddr_in6 *) runp->ai_addr; + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) + /* No scope ID present. */ + scope[0] = '\0'; + else + snprintf (scope, sizeof (scope), "%%%" PRIu32, + addr6->sin6_scope_id); + char buf[INET6_ADDRSTRLEN]; - printf ("%-15s %-6s %s\n", - inet_ntop (runp->ai_family, - runp->ai_family == AF_INET - ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, - buf, sizeof (buf)), - sockstr, - runp->ai_canonname ?: ""); + if (inet_ntop (runp->ai_family, + runp->ai_family == AF_INET + ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr + : &addr6->sin6_addr, + buf, sizeof (buf)) == NULL) + strcpy (buf, "<invalid>"); + + int pad = 15 - strlen (buf) - strlen (scope); + if (pad < 0) + pad = 0; + + printf ("%s%-*s %-6s %s\n", + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); runp = runp->ai_next; }
On 06/02/2019 14:52, Florian Weimer wrote: > * Adhemerval Zanella: > >>> @@ -393,15 +394,30 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) >>> sockstr = sockbuf; >>> } >>> >>> + char scope[3 * sizeof (uint32_t) + 2]; >> >> I would prefer something more explicit as: >> >> /* It requires to hold a uint32_t plus a '%'. */ >> cat scope[((CHAR_BIT * sizeof(uint32_t) / 3) + 3) + 1]; >> >> But I don't have a strong opinion here. > > “3 digits per byte” is sufficiently universal that I find your longer > version not particularly illuminating. 8-/ > > I have added a commented about the %. New patch below. > >>> + int pad = 15 - strlen (buf) - strlen (scope); >>> + if (pad < 0) >>> + pad = 0; >> >> Not fun of the magic constant, but it came from original code anyway. > > Right. > >>> + >>> + printf ("%s%-*s %-6s %s\n", >>> + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); >> >> Do we need to print the scope for invalid address as well? > > No, I think the invalid address can only happen (theoretically) for an > unexpected address family from getaddrinfo. Right, so in this case shouldn't we just print 'buf' without 'scope' then? > > Thanks, > Florian > > nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available > > 2019-02-06 Florian Weimer <fweimer@redhat.com> > > * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. LGTM, thanks. > > diff --git a/nss/getent.c b/nss/getent.c > index f25de8f1fc..df73d8fb29 100644 > --- a/nss/getent.c > +++ b/nss/getent.c > @@ -40,6 +40,7 @@ > #include <netinet/in.h> > #include <sys/socket.h> > #include <scratch_buffer.h> > +#include <inttypes.h> > > /* Get libc version number. */ > #include <version.h> > @@ -393,15 +394,31 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) > sockstr = sockbuf; > } > > + /* Three digits per byte, plus '%' and null terminator. */ > + char scope[3 * sizeof (uint32_t) + 2]; > + struct sockaddr_in6 *addr6 > + = (struct sockaddr_in6 *) runp->ai_addr; > + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) > + /* No scope ID present. */ > + scope[0] = '\0'; > + else > + snprintf (scope, sizeof (scope), "%%%" PRIu32, > + addr6->sin6_scope_id); > + > char buf[INET6_ADDRSTRLEN]; > - printf ("%-15s %-6s %s\n", > - inet_ntop (runp->ai_family, > - runp->ai_family == AF_INET > - ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, > - buf, sizeof (buf)), > - sockstr, > - runp->ai_canonname ?: ""); > + if (inet_ntop (runp->ai_family, > + runp->ai_family == AF_INET > + ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > + : &addr6->sin6_addr, > + buf, sizeof (buf)) == NULL) > + strcpy (buf, "<invalid>"); > + > + int pad = 15 - strlen (buf) - strlen (scope); > + if (pad < 0) > + pad = 0; > + > + printf ("%s%-*s %-6s %s\n", > + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); > > runp = runp->ai_next; > } >
* Adhemerval Zanella: >>>> + >>>> + printf ("%s%-*s %-6s %s\n", >>>> + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); >>> >>> Do we need to print the scope for invalid address as well? >> >> No, I think the invalid address can only happen (theoretically) for an >> unexpected address family from getaddrinfo. > > Right, so in this case shouldn't we just print 'buf' without 'scope' > then? Like this? I'll commit it after testing (manually). Thanks, Florian nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available This information is sometimes useful and actually required for link-local addresses. 2019-02-11 Florian Weimer <fweimer@redhat.com> * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. diff --git a/nss/getent.c b/nss/getent.c index f25de8f1fc..07a7d09795 100644 --- a/nss/getent.c +++ b/nss/getent.c @@ -40,6 +40,7 @@ #include <netinet/in.h> #include <sys/socket.h> #include <scratch_buffer.h> +#include <inttypes.h> /* Get libc version number. */ #include <version.h> @@ -393,15 +394,34 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) sockstr = sockbuf; } + /* Three digits per byte, plus '%' and null terminator. */ + char scope[3 * sizeof (uint32_t) + 2]; + struct sockaddr_in6 *addr6 + = (struct sockaddr_in6 *) runp->ai_addr; + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) + /* No scope ID present. */ + scope[0] = '\0'; + else + snprintf (scope, sizeof (scope), "%%%" PRIu32, + addr6->sin6_scope_id); + char buf[INET6_ADDRSTRLEN]; - printf ("%-15s %-6s %s\n", - inet_ntop (runp->ai_family, - runp->ai_family == AF_INET - ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, - buf, sizeof (buf)), - sockstr, - runp->ai_canonname ?: ""); + if (inet_ntop (runp->ai_family, + runp->ai_family == AF_INET + ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr + : &addr6->sin6_addr, + buf, sizeof (buf)) == NULL) + { + strcpy (buf, "<invalid>"); + scope[0] = '\0'; + } + + int pad = 15 - strlen (buf) - strlen (scope); + if (pad < 0) + pad = 0; + + printf ("%s%-*s %-6s %s\n", + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); runp = runp->ai_next; }
On 11/02/2019 10:52, Florian Weimer wrote: > * Adhemerval Zanella: > >>>>> + >>>>> + printf ("%s%-*s %-6s %s\n", >>>>> + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); >>>> >>>> Do we need to print the scope for invalid address as well? >>> >>> No, I think the invalid address can only happen (theoretically) for an >>> unexpected address family from getaddrinfo. >> >> Right, so in this case shouldn't we just print 'buf' without 'scope' >> then? > > Like this? > > I'll commit it after testing (manually). LGTM, thanks. > > Thanks, > Florian > > nss: getent: Print IPv6 scope ID for ahosts/ahostsv6 if available > > This information is sometimes useful and actually required for > link-local addresses. > > 2019-02-11 Florian Weimer <fweimer@redhat.com> > > * nss/getent.c (ahosts_keys_int): Include IPv6 scope ID in output. > > diff --git a/nss/getent.c b/nss/getent.c > index f25de8f1fc..07a7d09795 100644 > --- a/nss/getent.c > +++ b/nss/getent.c > @@ -40,6 +40,7 @@ > #include <netinet/in.h> > #include <sys/socket.h> > #include <scratch_buffer.h> > +#include <inttypes.h> > > /* Get libc version number. */ > #include <version.h> > @@ -393,15 +394,34 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) > sockstr = sockbuf; > } > > + /* Three digits per byte, plus '%' and null terminator. */ > + char scope[3 * sizeof (uint32_t) + 2]; > + struct sockaddr_in6 *addr6 > + = (struct sockaddr_in6 *) runp->ai_addr; > + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) > + /* No scope ID present. */ > + scope[0] = '\0'; > + else > + snprintf (scope, sizeof (scope), "%%%" PRIu32, > + addr6->sin6_scope_id); > + > char buf[INET6_ADDRSTRLEN]; > - printf ("%-15s %-6s %s\n", > - inet_ntop (runp->ai_family, > - runp->ai_family == AF_INET > - ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, > - buf, sizeof (buf)), > - sockstr, > - runp->ai_canonname ?: ""); > + if (inet_ntop (runp->ai_family, > + runp->ai_family == AF_INET > + ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr > + : &addr6->sin6_addr, > + buf, sizeof (buf)) == NULL) > + { > + strcpy (buf, "<invalid>"); > + scope[0] = '\0'; > + } > + > + int pad = 15 - strlen (buf) - strlen (scope); > + if (pad < 0) > + pad = 0; > + > + printf ("%s%-*s %-6s %s\n", > + buf, pad, scope, sockstr, runp->ai_canonname ?: ""); > > runp = runp->ai_next; > } >
diff --git a/nss/getent.c b/nss/getent.c index f25de8f1fc..1159ba2636 100644 --- a/nss/getent.c +++ b/nss/getent.c @@ -393,14 +393,24 @@ ahosts_keys_int (int af, int xflags, int number, char *key[]) sockstr = sockbuf; } + char scope_buf[3 * sizeof (unsigned long long int) + 2]; + struct sockaddr_in6 *addr6 + = (struct sockaddr_in6 *) runp->ai_addr; + if (runp->ai_family != AF_INET6 || addr6->sin6_scope_id == 0) + /* No scope ID present. */ + scope_buf[0] = '\0'; + else + snprintf (scope_buf, sizeof (scope_buf), "%%%llu", + (unsigned long long int) addr6->sin6_scope_id); + char buf[INET6_ADDRSTRLEN]; - printf ("%-15s %-6s %s\n", + printf ("%-15s%s %-6s %s\n", inet_ntop (runp->ai_family, runp->ai_family == AF_INET ? (void *) &((struct sockaddr_in *) runp->ai_addr)->sin_addr - : (void *) &((struct sockaddr_in6 *) runp->ai_addr)->sin6_addr, + : &addr6->sin6_addr, buf, sizeof (buf)), - sockstr, + scope_buf, sockstr, runp->ai_canonname ?: ""); runp = runp->ai_next;