From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta1.formilux.org (mta1.formilux.org [51.159.59.229]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3695C2DB7B7 for ; Sun, 8 Feb 2026 15:47:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=51.159.59.229 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770565647; cv=none; b=b27B4iudrBgyPu9YhqsAi39sm0tjogiz9FDOc3++id6SvK4dcFg/P+HtnFwlf+hBQCo1KHj5iVWlZ8nWPe17d0JFz9cOH8bCBzghKytnaf9eDh4CgbYcdLCu3srgXl2pGPo+kusJq0n+FLizEdDWPi3l9WPCA1pVS45g1XeFhbo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770565647; c=relaxed/simple; bh=mW5pfwblk5RG2BDO6mJWMsap9YooAh+ADrfj8ZXnQBs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=U4ypHJh8FWpBsvGdZZdW+bdau64McPpowEQJHvpOCa7yg4crYa5IAwXIXn/FIwHWZ85/OU/VUNh5GBtutpc87qIiPszCtwgxdklw+ab7FGrUsk50D+OcjcvK2lmaG459Widcu6TMiN3Mtm64qzpKHAsTfGgNgeU3XtNNANrf7xk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=1wt.eu; spf=pass smtp.mailfrom=1wt.eu; dkim=pass (1024-bit key) header.d=1wt.eu header.i=@1wt.eu header.b=kQlszvR2; arc=none smtp.client-ip=51.159.59.229 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=1wt.eu Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=1wt.eu Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=1wt.eu header.i=@1wt.eu header.b="kQlszvR2" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1wt.eu; s=mail; t=1770565644; bh=nsMM5dRyZzhcjLemGbAr0outmHxRLzcElDIb4jeQ1vg=; h=From:Message-ID:From; b=kQlszvR2hd0N3bgjTFnmOpx54gnQxdxhdfqK1RGH8bFqjDTONv5Fxh9os0XSJeYyk IJE6vk5i/fkzH2Z/UM+fn3DJUWdARgu6LU5/kE4qnkSuTxOlz7P3MjY+Ttdppy8jpe 6gQKJ4sZDEd4iNwOi/8wlqAd1Xi0p7xI0wD+fO90= Received: from 1wt.eu (ded1.1wt.eu [163.172.96.212]) by mta1.formilux.org (Postfix) with ESMTP id 12F65C0A70; Sun, 08 Feb 2026 16:47:24 +0100 (CET) Date: Sun, 8 Feb 2026 16:47:23 +0100 From: Willy Tarreau To: david.laight.linux@gmail.com Cc: Thomas =?iso-8859-1?Q?Wei=DFschuh?= , linux-kernel@vger.kernel.org, Cheng Li Subject: Re: [PATCH v2 next 07/11] tools/nolibc/printf: Add support for conversion flags "#- +" and format "%X" Message-ID: References: <20260206191121.3602-1-david.laight.linux@gmail.com> <20260206191121.3602-8-david.laight.linux@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260206191121.3602-8-david.laight.linux@gmail.com> On Fri, Feb 06, 2026 at 07:11:17PM +0000, david.laight.linux@gmail.com wrote: > -/* simple printf(). It supports the following formats: > - * - %[-][width][{l,t,z,ll,L,j,q}]{d,u,c,x,p,s,m,%} > - * - %% > - * - invalid formats are copied to the output buffer > +/* printf(). Supports most of the normal integer and string formats. > + * - %[#-+ ][width][{l,t,z,ll,L,j,q}]{d,i,u,c,x,X,p,s,m,%} > + * - %% generates a single % > + * - %m outputs strerror(errno). > + * - # only affects %x and prepends 0x to non-zero values. > + * - %o (octal) isn't supported. > + * - %X outputs a..f the same as %x. > + * - No support for zero padding, precision or variable widths. > + * - No support for wide characters. > + * - invalid formats are copied to the output buffer. > */ Thanks for updating this one, it does help quite a bit. > /* This code uses 'flag' variables that are indexed by the low 6 bits > @@ -279,7 +285,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list > unsigned int written, width; > unsigned int flags, ch_flag; > size_t len; > - char tmpbuf[21]; > + char tmpbuf[32 + 24]; The previous buffer was sized to store a 64-bit int. I couldn't figure what these 32 and 24 correspond to with the new supported specifiers. Maybe please add a short comment on the line to hint about what they correspond to ? > const char *outstr; > > written = 0; > @@ -334,19 +340,32 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list > > /* Conversion specifiers. */ > > - /* Numeric conversion specifiers. */ > - ch_flag = _NOLIBC_PF_CHAR_IS_ONE_OF(ch, 'c', 'd', 'i', 'u', 'x', 'p'); > - if (ch_flag != 0) { > + /* Numeric and pointer conversion specifiers. > + * > + * Use an explicit bound check (rather than _NOLIBC_PF_CHAR_IS_ONE_OF()) > + * so that 'X' can be allowed through. > + * 'X' gets treated and 'x' because _NOLIBC_PF_FLAG() returns the same > + * value for both. > + */ > + if ((ch < 'a' || ch > 'z') && ch != 'X') > + goto non_numeric_conversion; > + > + /* We need to check for "%p" or "%#x" later, merging here gives better code. > + * But '#' collides with 'c' so shift right. > + */ > + ch_flag = _NOLIBC_PF_FLAG(ch) | (flags & _NOLIBC_PF_FLAG('#')) >> 1; > + if (_NOLIBC_PF_FLAGS_CONTAIN(ch_flag, 'c', 'd', 'i', 'u', 'x', 'p', 's')) { > unsigned long long v; > long long signed_v; > - char *out = tmpbuf; > + char *out = tmpbuf + 32; OK so you seem to be reserving a part of the buffer for certain uses ? > + int sign = 0; > > /* 'long' is needed for pointer/string conversions and ltz lengths. > * A single test can be used provided 'p' (the same bit as '0') > * is masked from flags. > */ > if (_NOLIBC_PF_FLAGS_CONTAIN(ch_flag | (flags & ~_NOLIBC_PF_FLAG('p')), > - 'p', 'l', 't', 'z')) { > + 'p', 's', 'l', 't', 'z')) { > v = va_arg(args, unsigned long); > signed_v = (long)v; > } else if (_NOLIBC_PF_FLAGS_CONTAIN(flags, 'j', 'q')) { > @@ -365,40 +384,62 @@ int __nolibc_printf(__nolibc_printf_cb cb, void *state, const char *fmt, va_list > goto do_output; > } > > + if (_NOLIBC_PF_FLAGS_CONTAIN(ch_flag, 's')) { > + /* "%s" - character string. */ > + if (!v) { > + outstr = "(null)"; > + len = 6; > + goto do_output; > + } > + outstr = (void *)v; > +do_strnlen_output: > + len = strnlen(outstr, INT_MAX); I get why you turned strlen() to strnlen(INT_MAX) (result being an int) but this will not change anything IMHO in that the rest of a 2GB+ string will be written in multiple passes and will overflow the output anyway. Thus I think that sticking to strlen() remains simpler and less confusing. (...) > - else if (ch == 'm') { > + > +non_numeric_conversion: > + if (ch == 'm') { > #ifdef NOLIBC_IGNORE_ERRNO > outstr = "unknown error"; > + len = __builtin_strlen(outstr); > #else > outstr = strerror(errno); > + goto do_strnlen_output; > #endif /* NOLIBC_IGNORE_ERRNO */ It's simlper (and smaller) to use the common label for both here: #ifdef NOLIBC_IGNORE_ERRNO outstr = "unknown error"; #else outstr = strerror(errno); #endif /* NOLIBC_IGNORE_ERRNO */ + goto do_strnlen_output; Overall OK to me. Willy