30.10.2025 22:42, Jeremie Courreges-Anglas пишет:
> On Thu, Oct 30, 2025 at 05:49:15PM +0000, Klemens Nanni wrote:
>> If the argument contains a '%', the code will modify argv[] in place,
>> which causes rc.subr(8)'s pexp aka. pgrep(1) to mismatch and thus
>> rcctl(8) to report failure despite the service running fine:
>>
>> 	$ rcctl get iperf3 flags
>> 	-6 --bind fe80::1%vport0
>> 	$ rcctl check iperf3
>> 	iperf3(failed)
>> 	$ pgrep -fl iperf3
>> 	33091 /usr/local/bin/iperf3 -s -D -6 --bind fe80::1
>>
>>
>> Funnily, the code knows that:
>>
>> 	 * Modifies the string pointed to by spec in-place due to the use of
>> 		  * strtok(3). The caller should strdup(3) or otherwise copy the string
>> 	 * if an unmodified copy is needed.
>> 	 */
>> 	int
>> 	iperf_parse_hostname(struct iperf_test *test, char *spec, char **p, char **p1) {
>>
>>
>> Simply pass a copy to that function to keep scope identifies in argv[]:
>>
>> 	$ rcctl check iperf3
>> 	iperf3(ok)
>> 	$ pgrep -fl iperf3
>> 	98863 /usr/local/bin/iperf3 -s -D -6 --bind fe80::1%vport0
>>
>> --client works the same as --bind, so fix it as well.
>>
>> Feedback? OK?
> 
> Nit that isn't just a nit: I guess this is only used at startup, but
> I'd suggest to check the return value of strdup(3), just for
> correctness.

iperf3 strdup()s stuff all over the place without checking for NULL.
In most places it seems to store away the return value and use it later
through some getter function.

Here's an extra check (provoked with `arg = NULL, errno = ENOMEM;'),
but it does not match the rest of the code;  the -1 is needed to hit
iperf_strerror()'s default case:

	$ iperf3 -6 -s --bind fe80::1%lo0                                                 <	
	iperf3: parameter error - int_errno=-1: Cannot allocate memory
	
	Usage: iperf3 [-s|-c host] [options]
	Try `iperf3 --help' for more information.


I've left the upstream PR as-is, no response so far.

Feedback? OK for either of the two versions?


Index: Makefile
===================================================================
RCS file: /cvs/ports/net/iperf3/Makefile,v
diff -u -p -r1.21 Makefile
--- Makefile	26 Dec 2024 18:12:38 -0000	1.21
+++ Makefile	30 Oct 2025 17:08:08 -0000
@@ -3,6 +3,7 @@ COMMENT=	tool to measure maximum achieva
 V=		3.18
 DISTNAME=	iperf-${V}
 PKGNAME=	iperf3-${V}
+REVISION=	0
 
 SHARED_LIBS +=	iperf                5.0      # 0.0
 
Index: patches/patch-src_iperf_api_c
===================================================================
RCS file: /cvs/ports/net/iperf3/patches/patch-src_iperf_api_c,v
diff -u -p -r1.12 patch-src_iperf_api_c
--- patches/patch-src_iperf_api_c	26 Dec 2024 18:12:38 -0000	1.12
+++ patches/patch-src_iperf_api_c	7 Nov 2025 23:17:32 -0000
@@ -1,9 +1,74 @@
-Default to IPv4.
+- Pass a copy of argv[] to prevent changes by strtok(3),
+  otherwise rc.subr(8) pexp might not match and cause rcctl failure.
+  Pending https://github.com/esnet/iperf/pull/1960
+
+- Default to IPv4.
 
 Index: src/iperf_api.c
 --- src/iperf_api.c.orig
 +++ src/iperf_api.c
-@@ -2998,7 +2998,7 @@ iperf_defaults(struct iperf_test *testp)
+@@ -1243,7 +1243,7 @@ iperf_parse_arguments(struct iperf_test *test, int arg
+                 }
+ 		iperf_set_test_role(test, 's');
+                 break;
+-            case 'c':
++            case 'c': {
+                 if (test->role == 's') {
+                     i_errno = IESERVCLIENT;
+                     return -1;
+@@ -1251,7 +1251,12 @@ iperf_parse_arguments(struct iperf_test *test, int arg
+ 		iperf_set_test_role(test, 'c');
+ 		iperf_set_test_server_hostname(test, optarg);
+ 
+-                if (iperf_parse_hostname(test, optarg, &p, &p1)) {
++                char *arg = strdup(optarg);
++                if (arg == NULL) {
++                    i_errno = -1;
++                    return -1;
++                }
++                if (iperf_parse_hostname(test, arg, &p, &p1)) {
+ #if defined(HAVE_SO_BINDTODEVICE)
+                     /* Get rid of the hostname we saved earlier. */
+                     free(iperf_get_test_server_hostname(test));
+@@ -1262,7 +1267,9 @@ iperf_parse_arguments(struct iperf_test *test, int arg
+                     return -1;
+ #endif /* HAVE_SO_BINDTODEVICE */
+                 }
++                free(arg);
+                 break;
++            }
+             case 'u':
+                 set_protocol(test, Pudp);
+ 		client_flag = 1;
+@@ -1374,10 +1381,15 @@ iperf_parse_arguments(struct iperf_test *test, int arg
+ 		client_flag = 1;
+                 break;
+ 
+-            case 'B':
++            case 'B': {
+                 iperf_set_test_bind_address(test, optarg);
+ 
+-                if (iperf_parse_hostname(test, optarg, &p, &p1)) {
++                char *arg = strdup(optarg);
++                if (arg == NULL) {
++                    i_errno = -1;
++                    return -1;
++                }
++                if (iperf_parse_hostname(test, arg, &p, &p1)) {
+ #if defined(HAVE_SO_BINDTODEVICE)
+                     /* Get rid of the hostname we saved earlier. */
+                     free(iperf_get_test_bind_address(test));
+@@ -1388,7 +1400,9 @@ iperf_parse_arguments(struct iperf_test *test, int arg
+                     return -1;
+ #endif /* HAVE_SO_BINDTODEVICE */
+                 }
++                free(arg);
+                 break;
++            }
+ #if defined (HAVE_SO_BINDTODEVICE)
+             case OPT_BIND_DEV:
+                 iperf_set_test_bind_dev(test, optarg);
+@@ -2998,7 +3012,7 @@ iperf_defaults(struct iperf_test *testp)
      testp->stats_interval = testp->reporter_interval = 1;
      testp->num_streams = 1;