Re: [PATCH 3/3] tools/nolibc: add testcases for vfprintf

[Willy Tarreau]

On Sun, Apr 02, 2023 at 12:18:29PM +0000, Thomas Weißschuh wrote:
> On 2023-04-02 09:51:10+0200, Willy Tarreau wrote:
> > On Tue, Mar 28, 2023 at 09:01:31PM +0000, Thomas Weißschuh wrote:
> > > vfprintf() is complex and so far did not have proper tests.
> > 
> > This is an excellent idea, I totally agree, and I wouldn't be surprised
> > if there were still bugs there.
> 
> The first issue I experienced was that
> 
> printf("%*s", 1, "foo") would segfault because it ignored the '*' and
> just tried to interpret the number "1" as string.

Yes indeed, much like many older printf() implementations as well BTW,
that's a common issue when you try to write portable code ;-)

> When looking for the supported features of the printf implementation
> there were no examples.

Indeed!

> And before I try to add code to handle this case better I really want
> some testcases.
> 
> > > +		switch (test + __LINE__ + 1) {
> > > +		CASE_TEST(empty);        EXPECT_VFPRINTF(0, "", ""); break;
> > > +		CASE_TEST(simple);       EXPECT_VFPRINTF(3, "foo", "foo"); break;
> > > +		CASE_TEST(string);       EXPECT_VFPRINTF(3, "foo", "%s", "foo"); break;
> > > +		CASE_TEST(number);       EXPECT_VFPRINTF(4, "1234", "%d", 1234); break;
> > > +		CASE_TEST(negnumber);    EXPECT_VFPRINTF(5, "-1234", "%d", -1234); break;
> > > +		CASE_TEST(unsigned);     EXPECT_VFPRINTF(5, "12345", "%u", 12345); break;
> > > +		CASE_TEST(char);         EXPECT_VFPRINTF(1, "c", "%c", 'c'); break;
> > > +		CASE_TEST(hex);          EXPECT_VFPRINTF(1, "f", "%x", 0xf); break;
> > > +		CASE_TEST(pointer);      EXPECT_VFPRINTF(3, "0x0", "%p", NULL); break;
> > 
> > I don't see a reason why not to move them to the stdlib category, since
> > these tests are there to validate that the libc-provided functions do
> > work. Maybe you intended to further extend it ? In this case maybe we
> > could move that to an "stdio" category then but I'd rather avoid having
> > one category per function or it will quickly become annoying to select
> > groups of tests. So let's just prefix these test names with "printf_"
> > and either merge them with "stdlib" or name the category "stdio", as
> > you prefer.
> 
> The idea was that printf is its own very special beast that alone is
> more complex than many other things combined.
> When working on it, it would be useful to only run the relevant tests
> without having to manually count testcase numbers.
> 
> I don't expect other single functions getting their own category.
> 
> If you still prefer to put it somewhere else I can do that, too.

OK, I can understand, it makes sense to some extents. And I agree that
if we'd ever extend printf it would be needed to extend these tests.
Then let's leave it that way.

Thanks,
Willy