Re: git: dbaaadd4373a - main - jls: minor simplification to arg handling
- In reply to: James Gritton : "Re: git: dbaaadd4373a - main - jls: minor simplification to arg handling"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 15 Sep 2025 11:49:03 UTC
On Mon, Sep 15, 2025, at 01:05, James Gritton wrote:
> On 2025-09-14 19:30, Kyle Evans wrote:
>> On 9/14/25 20:00, Kyle Evans wrote:
>>> On 9/14/25 19:14, Danilo G. Baio wrote:
>>>>
>>>>
>>>> On Sat, Jul 26, 2025, at 00:14, Kyle Evans wrote:
>>>>> The branch main has been updated by kevans:
>>>>>
>>>>> URL:
>>>>> https://cgit.FreeBSD.org/src/commit/?id=dbaaadd4373a725950ad11e578dab61537b7c4f2
>>>>>
>>>>> commit dbaaadd4373a725950ad11e578dab61537b7c4f2
>>>>> Author: Kyle Evans <kevans@FreeBSD.org>
>>>>> AuthorDate: 2025-07-26 03:13:41 +0000
>>>>> Commit: Kyle Evans <kevans@FreeBSD.org>
>>>>> CommitDate: 2025-07-26 03:13:41 +0000
>>>>>
>>>>> jls: minor simplification to arg handling
>>>>> It's easier to reason about the state of argc/argv if we just
>>>>> augment
>>>>> them by optind after our getopt() loop.
>>>>> No functional change, but this sets the stage for another
>>>>> change to add
>>>>> a `-c` mode to (c)heck for the existence of a jail quietly
>>>>> without
>>>>> the caller having to worry about spurious output.
>>>>> Reviewed by: jamie
>>>>> Differential Revision: https://reviews.freebsd.org/D51540
>>>>> ---
>>>>> usr.sbin/jls/jls.c | 10 ++++++----
>>>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/usr.sbin/jls/jls.c b/usr.sbin/jls/jls.c
>>>>> index bd193a69c458..a1d1716713aa 100644
>>>>> --- a/usr.sbin/jls/jls.c
>>>>> +++ b/usr.sbin/jls/jls.c
>>>>> @@ -140,8 +140,11 @@ main(int argc, char **argv)
>>>>> ip4_ok = feature_present("inet");
>>>>> #endif
>>>>>
>>>>> + argc -= optind;
>>>>> + argv += optind;
>>>>> +
>>>>> /* Add the parameters to print. */
>>>>> - if (optind == argc) {
>>>>> + if (argc == 0) {
>>>>> if (pflags & (PRINT_HEADER | PRINT_NAMEVAL))
>>>>> add_param("all", NULL, (size_t)0, NULL, JP_USER);
>>>>> else if (pflags & PRINT_VERBOSE) {
>>>>> @@ -179,9 +182,8 @@ main(int argc, char **argv)
>>>>> }
>>>>> } else {
>>>>> pflags &= ~PRINT_VERBOSE;
>>>>> - while (optind < argc)
>>>>> - add_param(argv[optind++], NULL, (size_t)0, NULL,
>>>>> - JP_USER);
>>>>> + for (i = 0; i < argc; i++)
>>>>> + add_param(argv[i], NULL, (size_t)0, NULL, JP_USER);
>>>>> }
>>>>>
>>>>> if (pflags & PRINT_SKIP) {
>>>>
>>>>
>>>> Hi,
>>>>
>>>> Just replying to one of the recent changes on `jls`.
>>>>
>>>> We use `jls -n` in many scripts, and recently, it stopped working.
>>>>
>>>> The last build that was working for us:
>>>> FreeBSD 15.0-CURRENT #0 main-n278879-4be9c6f38e78: Sat Jul 19
>>>> 13:19:25 UTC 2025
>>>>
>>>> We are now encountering the following issue on this build:
>>>> FreeBSD 16.0-CURRENT #0 main-n280141-5e82eeccd252: Sat Sep 6
>>>> 05:27:34 UTC 2025
>>>>
>>>> $ jls -n
>>>> desc=0 devfs_ruleset=0 nodying enforce_statfs=0 env="" host=disable
>>>> ip4=disable ip6=disable jid=0 meta="" name="" osreldate=0
>>>> osrelease="" parent=0 path="" nopersist securelevel=0 sysvmsg=disable
>>>> sysvsem=disable sysvshm=disable vnet=disable zfs=disable
>>>> allow.noadjtime allow.nochflags allow.noextattr allow.nomlock
>>>> allow.nomount allow.mount.nodevfs allow.mount.nofdescfs
>>>> allow.mount.nonullfs allow.mount.noprocfs allow.mount.notmpfs
>>>> allow.mount.nozfs allow.nonfsd allow.noquotas allow.noraw_sockets
>>>> allow.noread_msgbuf allow.noreserved_ports allow.norouting
>>>> allow.noset_hostname allow.nosettime allow.nosocket_af allow.nosuser
>>>> allow.nosysvipc allow.nounprivileged_parent_tampering
>>>> allow.nounprivileged_proc_debug children.cur=0 children.max=0
>>>> cpuset.id=0 host.domainname="" host.hostid=0 host.hostname=""
>>>> host.hostuuid=""
>>>> ip4.addr=0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0
>>>> ip4.nosaddrsel ip6.addr=::,::,::,::,::,:: ip6.nosaddrsel
>>>> zfs.mount_snapshot=0
>>>> desc=0 devfs_ruleset=0 nodying enforce_statfs=0 env="" host=disable
>>>> ip4=disable ip6=disable jid=0 meta="" name="" osreldate=0
>>>> osrelease="" parent=0 path="" nopersist securelevel=0 sysvmsg=disable
>>>> sysvsem=disable sysvshm=disable vnet=disable zfs=disable
>>>> allow.noadjtime allow.nochflags allow.noextattr allow.nomlock
>>>> allow.nomount allow.mount.nodevfs allow.mount.nofdescfs
>>>> allow.mount.nonullfs allow.mount.noprocfs allow.mount.notmpfs
>>>> allow.mount.nozfs allow.nonfsd allow.noquotas allow.noraw_sockets
>>>> allow.noread_msgbuf allow.noreserved_ports allow.norouting
>>>> allow.noset_hostname allow.nosettime allow.nosocket_af allow.nosuser
>>>> allow.nosysvipc allow.nounprivileged_parent_tampering
>>>> allow.nounprivileged_proc_debug children.cur=0 children.max=0
>>>> cpuset.id=0 host.domainname="" host.hostid=0 host.hostname=""
>>>> host.hostuuid=""
>>>> ip4.addr=0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0,0.0.0.0
>>>> ip4.nosaddrsel ip6.addr=::,::,::,::,::,:: ip6.nosaddrsel
>>>> zfs.mount_snapshot=0
>>>> desc=0 devfs_ruleset=0 nodying enforce_statfs=0 env="" host=disable
>>>> ip4=disable ip6=disable jid=0 meta="" name="" osreldate=0
>>>> osrelease="" parent=0 path="" nopersist securelevel=0 sysvmsg=disable
>>>> sysvsem=disable sysvshm=disable vnet=disable zfs=disable
>>>> allow.noadjtime allow.nochflags allow.noextattr allow.nomlock
>>>> allow.nomount allow.mount.nodevfs allow.mount.nofdescfs
>>>> allow.mount.nonullfs allow.mount.noprocfs allow.mount.notmpfs
>>>> allow.mount.nozfs allow.nonfsd allow.noquotas allow.noraw_sockets
>>>> allow.noread_msgbuf allow.noreserved_ports allow.norouting
>>>> allow.noset_hostname allow.nosettime
>>>> [...]
>>>> infinite loop
>>>>
>>>
>>> Adding jamie@, neither of the changes to jls(1) should've caused this,
>>> as far as I can reason about (and I haven't observed this here, yet).
>>> This looks like a failure to terminate the print_jail loop at the end?
>>
>> I still can't functionally update, but looking at the recent jaildesc
>> work, I wonder
>> if this is a side effect that has since been fixed in
>> e75dda31c1eead9ad40580bd8 by
>> removing the "desc" parameter. It would have been included in
>> jailparam_all(), and
>> I wonder if that somehow broke jls-style iteration?
>
> I've verified that the patch stocks the problem, and took the liberty of
> MFCing it a day early.
>
> - Jamie
Thanks to both of you for taking a look at this.
--
Danilo G. Baio