From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933950AbcBEAGZ (ORCPT ); Thu, 4 Feb 2016 19:06:25 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:33358 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933739AbcBEAGY (ORCPT ); Thu, 4 Feb 2016 19:06:24 -0500 From: Rasmus Villemoes To: "Jason A. Donenfeld" Cc: Joe Perches , Daniel Borkmann , Andrew Morton , andriy.shevchenko@linux.intel.com, LKML Subject: Re: [PATCH v2] vsprintf: automatic parameters for %pIS via 'a' Organization: D03 References: <1454540721.7291.141.camel@perches.com> <1454542185-1147-1-git-send-email-Jason@zx2c4.com> X-Hashcash: 1:20:160205:daniel@iogearbox.net::mzucy7Wl6knNz9OZ:0000000000000000000000000000000000000000006/k X-Hashcash: 1:20:160205:akpm@linux-foundation.org::6wxpVNZXm9dmb2VJ:0000000000000000000000000000000000000zr/ X-Hashcash: 1:20:160205:joe@perches.com::GWd1lJKGApWf4kZi:001Knb X-Hashcash: 1:20:160205:jason@zx2c4.com::rIHHg9ugqa3eKEVw:005YC3 X-Hashcash: 1:20:160205:andriy.shevchenko@linux.intel.com::ugKIuNGX7GTMiqJI:000000000000000000000000000040Q/ X-Hashcash: 1:20:160205:linux-kernel@vger.kernel.org::JeMAWlVuYcHxbEVv:0000000000000000000000000000000007NaM Date: Fri, 05 Feb 2016 01:06:16 +0100 In-Reply-To: <1454542185-1147-1-git-send-email-Jason@zx2c4.com> (Jason A. Donenfeld's message of "Thu, 4 Feb 2016 00:29:45 +0100") Message-ID: <87y4b0m35z.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 04 2016, "Jason A. Donenfeld" wrote: > This patch adds a variable 'a' which indicates that the 'p', > 'f', and 's' options should be toggled on or off depending on > whether or not those parameters are actually valid inside the > passed sockaddr. > > Signed-off-by: Jason A. Donenfeld > --- > Documentation/printk-formats.txt | 6 ++++-- > lib/vsprintf.c | 19 +++++++++++++++++-- > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt > index 5d1128b..22bae97 100644 > --- a/Documentation/printk-formats.txt > +++ b/Documentation/printk-formats.txt > @@ -193,7 +193,7 @@ IPv4/IPv6 addresses (generic, with port, flowinfo, scope): > %piS 001.002.003.004 or 00010002000300040005000600070008 > %pISc 1.2.3.4 or 1:2:3:4:5:6:7:8 > %pISpc 1.2.3.4:12345 or [1:2:3:4:5:6:7:8]:12345 > - %p[Ii]S[pfschnbl] > + %p[Ii]S[pfsachnbl] > > For printing an IP address without the need to distinguish whether it's > of type AF_INET or AF_INET6, a pointer to a valid 'struct sockaddr', > @@ -201,7 +201,9 @@ IPv4/IPv6 addresses (generic, with port, flowinfo, scope): > > The additional 'p', 'f', and 's' specifiers are used to specify port > (IPv4, IPv6), flowinfo (IPv6) and scope (IPv6). Ports have a ':' prefix, > - flowinfo a '/' and scope a '%', each followed by the actual value. > + flowinfo a '/' and scope a '%', each followed by the actual value. If 'a' > + is given, 'p', 'f', and 's' are activated or deactivated depending on > + whether or not the sockaddr has a non-zero port, flowinfo, and scope. > > In case of an IPv6 address the compressed IPv6 address as described by > http://tools.ietf.org/html/rfc5952 is being used if the additional > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 48ff9c3..9a07284 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -1145,7 +1145,7 @@ static noinline_for_stack > char *ip6_addr_string_sa(char *buf, char *end, const struct sockaddr_in6 *sa, > struct printf_spec spec, const char *fmt) > { > - bool have_p = false, have_s = false, have_f = false, have_c = false; > + bool have_p = false, have_s = false, have_f = false, have_c = false, have_a = false; > char ip6_addr[sizeof("[xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:255.255.255.255]") + > sizeof(":12345") + sizeof("/123456789") + > sizeof("%1234567890")]; > @@ -1169,9 +1169,18 @@ char *ip6_addr_string_sa(char *buf, char *end, const struct sockaddr_in6 *sa, > case 'c': > have_c = true; > break; > + case 'a': > + have_a = true; > + break; > } > } > > + if (have_a) { > + have_p = sa->sin6_port; > + have_s = sa->sin6_scope_id; > + have_f = sa->sin6_flowinfo & IPV6_FLOWINFO_MASK; > + } > + First of all I don't think vsprintf.c should be extended with yet another piece of unused code, so I hope there's an actual user coming. Second, please add tests to lib/test_printf.c. Third, wouldn't it be more convenient/flexible if a meant "print those that are present _and_ requested"; that is, the above would be > + have_p = have_p && sa->sin6_port; > + have_s = have_s && sa->sin6_scope_id; > + have_f = have_f && sa->sin6_flowinfo & IPV6_FLOWINFO_MASK; that way one could say %pISaf and have the flowinfo if and only if it's present, without also unintentionally turning on port or scope_id. Rasmus