Page MenuHomeFreeBSD
Differential D41743

libc: Add test cases for N2680.
ClosedPublic
Actions

Authored by des on Sep 5 2023, 6:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 15, 3:46 AM
Unknown Object (File)
Mon, Apr 14, 3:44 PM
Unknown Object (File)
Sun, Apr 13, 7:22 PM
Unknown Object (File)
Sat, Mar 29, 7:28 AM
Unknown Object (File)
Feb 23 2025, 6:42 PM
Unknown Object (File)
Jan 22 2025, 10:30 AM
Unknown Object (File)
Jan 17 2025, 8:09 PM
Unknown Object (File)
Jan 15 2025, 11:38 PM
View All 58 Files
Subscribers
imp

Details

Reviewers
imp
Group Reviewers
tests
Commits
rG12b1c1e3fb44: libc: Add test cases for N2680.
Summary

This adds test cases for %wN and %wfN to the printf(3) and scanf(3) tests.

While here, fix a few nits in the N2630 test cases.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 53474
Build 50365: arc lint + arc unit

Event Timeline

des created this revision.Sep 5 2023, 6:56 PM
Herald added a subscriber: imp. · View Herald TranscriptSep 5 2023, 6:56 PM
des requested review of this revision.Sep 5 2023, 6:56 PM
Harbormaster completed remote builds in B53449: Diff 126906.Sep 5 2023, 6:57 PM
des added parent revisions: D41725: libc: Implement N2680., D41726: libc: Add a wide version of snprintf_test., D41727: libc: Suppress format checks on printf() / scanf() tests..Sep 5 2023, 6:57 PM
des added a reviewer: tests.Sep 6 2023, 3:17 PM
imp accepted this revision.Sep 6 2023, 5:40 PM

We had some poorly formed issues with 0x and 0b numbers you added tests for.
Do you want to do the same for poorly formatted formats like %w88d?

This revision is now accepted and ready to land.Sep 6 2023, 5:40 PM
des added a comment.Sep 6 2023, 5:41 PM
In D41743#951667, @imp wrote:

We had some poorly formed issues with 0x and 0b numbers you added tests for.
Do you want to do the same for poorly formatted formats like %w88d?

You're right, I'll get right on it.

des updated this revision to Diff 126978.Sep 6 2023, 6:00 PM

add failure cases

This revision now requires review to proceed.Sep 6 2023, 6:00 PM
Harbormaster completed remote builds in B53474: Diff 126978.Sep 6 2023, 6:00 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 7 2023, 6:42 AM
Closed by commit rG12b1c1e3fb44: libc: Add test cases for N2680. (authored by des). · Explain Why
This revision was automatically updated to reflect the committed changes.
des added a commit: rG12b1c1e3fb44: libc: Add test cases for N2680..
des mentioned this in rG3c96ab9f1d28: libc: Implement N2680..Sep 7 2023, 8:29 PM

Revision Contents
Changeset List

PathSize
lib/
libc/
tests/
stdio/
66 lines
65 lines
66 lines
73 lines

Diff 126978

View Options

lib/libc/tests/stdio/snprintf_test.c

Show First 20 Lines • Show All 530 Lines • ▼ Show 20 Lines if (vs2 != NULL)
TAILQ_INSERT_BEFORE(vs2, vs, link); TAILQ_INSERT_BEFORE(vs2, vs, link);
else else
TAILQ_INSERT_TAIL(&vnet_constructors, vs, link); TAILQ_INSERT_TAIL(&vnet_constructors, vs, link);
/* /*
* Invoke the constructor on all the existing vnets when it is * Invoke the constructor on all the existing vnets when it is
* registered. * registered.
*/ */
VNET_LIST_RLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
void void
vnet_deregister_sysinit(void *arg) vnet_deregister_sysinit(void *arg)
{ {
struct vnet_sysinit *vs; struct vnet_sysinit *vs;
Show All 34 Linesvnet_deregister_sysuninit(void *arg)
struct vnet *vnet; struct vnet *vnet;
vs = arg; vs = arg;
/* /*
* Invoke the destructor on all the existing vnets when it is * Invoke the destructor on all the existing vnets when it is
* deregistered. * deregistered.
*/ */
VNET_LIST_RLOCK();
zleiUnsubmitted
Not Done
Inline Actions

In vnet_register_sysinit() we have lock order

VNET_SYSINIT_WLOCK();
VNET_LIST_RLOCK;

I think it is best to make lock order consistent with that in vnet_register_sysinit(), although dead lock is not possible.

zlei: In `vnet_register_sysinit()` we have lock order ``` VNET_SYSINIT_WLOCK(); VNET_LIST_RLOCK; ```…
VNET_SYSINIT_WLOCK(); VNET_SYSINIT_WLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
/* Remove the destructor from the global list of vnet destructors. */ /* Remove the destructor from the global list of vnet destructors. */
TAILQ_REMOVE(&vnet_destructors, vs, link); TAILQ_REMOVE(&vnet_destructors, vs, link);
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
mjgUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

the VNET_FOREACH macro should presumably assert on the lock?

mjg: you should unlock in the same order for consistency the VNET_FOREACH macro should presumably…
kpAuthorUnsubmitted
Not Done
Inline Actions

It should, but the locking for the vnet list uses two different locks for reasons I do not currently understand.
So we'd have to MPASS(sx_locked(&vnet_sxlock) || rw_rowned(&vnet_rwlock));. Which is doubly annoying because rw_rowned() doesn't exist.

kp: It should, but the locking for the vnet list uses two different locks for reasons I do not…
mjgUnsubmitted
Not Done
Inline Actions

that is a gap i'm going to plug

mjg: that is a gap i'm going to plug
zleiUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

I'd prefer to unlock in the reverse order. That is more natural for me.
Anyway unlock order does not matter much.

the VNET_FOREACH macro should presumably assert on the lock?

ipreass_drain() in sys/netinet/ip_reass.c also miss read lock.
I think it is good to have read lock assert in macro VNET_FOREACH .

zlei: > you should unlock in the same order for consistency I'd prefer to unlock in the reverse…
/* /*
* Invoke all registered vnet constructors on the current vnet. Used during * Invoke all registered vnet constructors on the current vnet. Used during
* vnet construction. The caller is responsible for ensuring the new vnet is * vnet construction. The caller is responsible for ensuring the new vnet is
* the current vnet and that the vnet_sysinit_sxlock lock is locked. * the current vnet and that the vnet_sysinit_sxlock lock is locked.
*/ */
static void static void
vnet_sysinit(void) vnet_sysinit(void)
▲ Show 20 Lines • Show All 235 Lines • Show Last 20 Lines
View Options

lib/libc/tests/stdio/sscanf_test.c

Show First 20 Lines • Show All 530 Lines • ▼ Show 20 Lines if (vs2 != NULL)
TAILQ_INSERT_BEFORE(vs2, vs, link); TAILQ_INSERT_BEFORE(vs2, vs, link);
else else
TAILQ_INSERT_TAIL(&vnet_constructors, vs, link); TAILQ_INSERT_TAIL(&vnet_constructors, vs, link);
/* /*
* Invoke the constructor on all the existing vnets when it is * Invoke the constructor on all the existing vnets when it is
* registered. * registered.
*/ */
VNET_LIST_RLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
void void
vnet_deregister_sysinit(void *arg) vnet_deregister_sysinit(void *arg)
{ {
struct vnet_sysinit *vs; struct vnet_sysinit *vs;
Show All 34 Linesvnet_deregister_sysuninit(void *arg)
struct vnet *vnet; struct vnet *vnet;
vs = arg; vs = arg;
/* /*
* Invoke the destructor on all the existing vnets when it is * Invoke the destructor on all the existing vnets when it is
* deregistered. * deregistered.
*/ */
VNET_LIST_RLOCK();
zleiUnsubmitted
Not Done
Inline Actions

In vnet_register_sysinit() we have lock order

VNET_SYSINIT_WLOCK();
VNET_LIST_RLOCK;

I think it is best to make lock order consistent with that in vnet_register_sysinit(), although dead lock is not possible.

zlei: In `vnet_register_sysinit()` we have lock order ``` VNET_SYSINIT_WLOCK(); VNET_LIST_RLOCK; ```…
VNET_SYSINIT_WLOCK(); VNET_SYSINIT_WLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
/* Remove the destructor from the global list of vnet destructors. */ /* Remove the destructor from the global list of vnet destructors. */
TAILQ_REMOVE(&vnet_destructors, vs, link); TAILQ_REMOVE(&vnet_destructors, vs, link);
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
mjgUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

the VNET_FOREACH macro should presumably assert on the lock?

mjg: you should unlock in the same order for consistency the VNET_FOREACH macro should presumably…
kpAuthorUnsubmitted
Not Done
Inline Actions

It should, but the locking for the vnet list uses two different locks for reasons I do not currently understand.
So we'd have to MPASS(sx_locked(&vnet_sxlock) || rw_rowned(&vnet_rwlock));. Which is doubly annoying because rw_rowned() doesn't exist.

kp: It should, but the locking for the vnet list uses two different locks for reasons I do not…
mjgUnsubmitted
Not Done
Inline Actions

that is a gap i'm going to plug

mjg: that is a gap i'm going to plug
zleiUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

I'd prefer to unlock in the reverse order. That is more natural for me.
Anyway unlock order does not matter much.

the VNET_FOREACH macro should presumably assert on the lock?

ipreass_drain() in sys/netinet/ip_reass.c also miss read lock.
I think it is good to have read lock assert in macro VNET_FOREACH .

zlei: > you should unlock in the same order for consistency I'd prefer to unlock in the reverse…
/* /*
* Invoke all registered vnet constructors on the current vnet. Used during * Invoke all registered vnet constructors on the current vnet. Used during
* vnet construction. The caller is responsible for ensuring the new vnet is * vnet construction. The caller is responsible for ensuring the new vnet is
* the current vnet and that the vnet_sysinit_sxlock lock is locked. * the current vnet and that the vnet_sysinit_sxlock lock is locked.
*/ */
static void static void
vnet_sysinit(void) vnet_sysinit(void)
▲ Show 20 Lines • Show All 235 Lines • Show Last 20 Lines
View Options

lib/libc/tests/stdio/swprintf_test.c

Show First 20 Lines • Show All 530 Lines • ▼ Show 20 Lines if (vs2 != NULL)
TAILQ_INSERT_BEFORE(vs2, vs, link); TAILQ_INSERT_BEFORE(vs2, vs, link);
else else
TAILQ_INSERT_TAIL(&vnet_constructors, vs, link); TAILQ_INSERT_TAIL(&vnet_constructors, vs, link);
/* /*
* Invoke the constructor on all the existing vnets when it is * Invoke the constructor on all the existing vnets when it is
* registered. * registered.
*/ */
VNET_LIST_RLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
void void
vnet_deregister_sysinit(void *arg) vnet_deregister_sysinit(void *arg)
{ {
struct vnet_sysinit *vs; struct vnet_sysinit *vs;
Show All 34 Linesvnet_deregister_sysuninit(void *arg)
struct vnet *vnet; struct vnet *vnet;
vs = arg; vs = arg;
/* /*
* Invoke the destructor on all the existing vnets when it is * Invoke the destructor on all the existing vnets when it is
* deregistered. * deregistered.
*/ */
VNET_LIST_RLOCK();
zleiUnsubmitted
Not Done
Inline Actions

In vnet_register_sysinit() we have lock order

VNET_SYSINIT_WLOCK();
VNET_LIST_RLOCK;

I think it is best to make lock order consistent with that in vnet_register_sysinit(), although dead lock is not possible.

zlei: In `vnet_register_sysinit()` we have lock order ``` VNET_SYSINIT_WLOCK(); VNET_LIST_RLOCK; ```…
VNET_SYSINIT_WLOCK(); VNET_SYSINIT_WLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
/* Remove the destructor from the global list of vnet destructors. */ /* Remove the destructor from the global list of vnet destructors. */
TAILQ_REMOVE(&vnet_destructors, vs, link); TAILQ_REMOVE(&vnet_destructors, vs, link);
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
mjgUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

the VNET_FOREACH macro should presumably assert on the lock?

mjg: you should unlock in the same order for consistency the VNET_FOREACH macro should presumably…
kpAuthorUnsubmitted
Not Done
Inline Actions

It should, but the locking for the vnet list uses two different locks for reasons I do not currently understand.
So we'd have to MPASS(sx_locked(&vnet_sxlock) || rw_rowned(&vnet_rwlock));. Which is doubly annoying because rw_rowned() doesn't exist.

kp: It should, but the locking for the vnet list uses two different locks for reasons I do not…
mjgUnsubmitted
Not Done
Inline Actions

that is a gap i'm going to plug

mjg: that is a gap i'm going to plug
zleiUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

I'd prefer to unlock in the reverse order. That is more natural for me.
Anyway unlock order does not matter much.

the VNET_FOREACH macro should presumably assert on the lock?

ipreass_drain() in sys/netinet/ip_reass.c also miss read lock.
I think it is good to have read lock assert in macro VNET_FOREACH .

zlei: > you should unlock in the same order for consistency I'd prefer to unlock in the reverse…
/* /*
* Invoke all registered vnet constructors on the current vnet. Used during * Invoke all registered vnet constructors on the current vnet. Used during
* vnet construction. The caller is responsible for ensuring the new vnet is * vnet construction. The caller is responsible for ensuring the new vnet is
* the current vnet and that the vnet_sysinit_sxlock lock is locked. * the current vnet and that the vnet_sysinit_sxlock lock is locked.
*/ */
static void static void
vnet_sysinit(void) vnet_sysinit(void)
▲ Show 20 Lines • Show All 235 Lines • Show Last 20 Lines
View Options

lib/libc/tests/stdio/swscanf_test.c

Show First 20 Lines • Show All 530 Lines • ▼ Show 20 Lines if (vs2 != NULL)
TAILQ_INSERT_BEFORE(vs2, vs, link); TAILQ_INSERT_BEFORE(vs2, vs, link);
else else
TAILQ_INSERT_TAIL(&vnet_constructors, vs, link); TAILQ_INSERT_TAIL(&vnet_constructors, vs, link);
/* /*
* Invoke the constructor on all the existing vnets when it is * Invoke the constructor on all the existing vnets when it is
* registered. * registered.
*/ */
VNET_LIST_RLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
void void
vnet_deregister_sysinit(void *arg) vnet_deregister_sysinit(void *arg)
{ {
struct vnet_sysinit *vs; struct vnet_sysinit *vs;
Show All 34 Linesvnet_deregister_sysuninit(void *arg)
struct vnet *vnet; struct vnet *vnet;
vs = arg; vs = arg;
/* /*
* Invoke the destructor on all the existing vnets when it is * Invoke the destructor on all the existing vnets when it is
* deregistered. * deregistered.
*/ */
VNET_LIST_RLOCK();
zleiUnsubmitted
Not Done
Inline Actions

In vnet_register_sysinit() we have lock order

VNET_SYSINIT_WLOCK();
VNET_LIST_RLOCK;

I think it is best to make lock order consistent with that in vnet_register_sysinit(), although dead lock is not possible.

zlei: In `vnet_register_sysinit()` we have lock order ``` VNET_SYSINIT_WLOCK(); VNET_LIST_RLOCK; ```…
VNET_SYSINIT_WLOCK(); VNET_SYSINIT_WLOCK();
VNET_FOREACH(vnet) { VNET_FOREACH(vnet) {
CURVNET_SET_QUIET(vnet); CURVNET_SET_QUIET(vnet);
vs->func(vs->arg); vs->func(vs->arg);
CURVNET_RESTORE(); CURVNET_RESTORE();
} }
/* Remove the destructor from the global list of vnet destructors. */ /* Remove the destructor from the global list of vnet destructors. */
TAILQ_REMOVE(&vnet_destructors, vs, link); TAILQ_REMOVE(&vnet_destructors, vs, link);
VNET_LIST_RUNLOCK();
VNET_SYSINIT_WUNLOCK(); VNET_SYSINIT_WUNLOCK();
} }
mjgUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

the VNET_FOREACH macro should presumably assert on the lock?

mjg: you should unlock in the same order for consistency the VNET_FOREACH macro should presumably…
kpAuthorUnsubmitted
Not Done
Inline Actions

It should, but the locking for the vnet list uses two different locks for reasons I do not currently understand.
So we'd have to MPASS(sx_locked(&vnet_sxlock) || rw_rowned(&vnet_rwlock));. Which is doubly annoying because rw_rowned() doesn't exist.

kp: It should, but the locking for the vnet list uses two different locks for reasons I do not…
mjgUnsubmitted
Not Done
Inline Actions

that is a gap i'm going to plug

mjg: that is a gap i'm going to plug
zleiUnsubmitted
Not Done
Inline Actions

you should unlock in the same order for consistency

I'd prefer to unlock in the reverse order. That is more natural for me.
Anyway unlock order does not matter much.

the VNET_FOREACH macro should presumably assert on the lock?

ipreass_drain() in sys/netinet/ip_reass.c also miss read lock.
I think it is good to have read lock assert in macro VNET_FOREACH .

zlei: > you should unlock in the same order for consistency I'd prefer to unlock in the reverse…
/* /*
* Invoke all registered vnet constructors on the current vnet. Used during * Invoke all registered vnet constructors on the current vnet. Used during
* vnet construction. The caller is responsible for ensuring the new vnet is * vnet construction. The caller is responsible for ensuring the new vnet is
* the current vnet and that the vnet_sysinit_sxlock lock is locked. * the current vnet and that the vnet_sysinit_sxlock lock is locked.
*/ */
static void static void
vnet_sysinit(void) vnet_sysinit(void)
▲ Show 20 Lines • Show All 235 Lines • Show Last 20 Lines