On Friday, 18 October 2024 23:47:13 CEST Noah Peterson wrote:

Modify the batctl ping utility to accept both integer and floating-point values for the interval between sending pings. This enhancement allows specifying intervals with millisecond precision.

You use nanosleep - so this would be nanoseconds. But it is unlikely that we would be able to send in this precise interval. So maybe rename it it subsecond.

For example: `sudo batctl ping aa:bb:cc:dd:ee:ff -i 0.5`

Also, improve error handling for invalid interval arguments.

Signed-off-by: Noah Peterson noahbpeterson1997@gmail.com

ping.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/ping.c b/ping.c index 3681e7e..104b17c 100644 --- a/ping.c +++ b/ping.c @@ -16,6 +16,7 @@ #include <signal.h> #include <fcntl.h> #include <string.h> +#include <limits.h> #include <math.h> #include <stddef.h> #include <stdint.h>

#include <time.h> missing after <sys/time.h>.

@@ -65,14 +66,15 @@ static int ping(struct state *state, int argc, char **argv) struct bat_host *bat_host, *rr_host; ssize_t read_len; int ret = EXIT_FAILURE, res, optchar, found_args = 1;

• int loop_count = -1, loop_interval = 0, timeout = 1, rr = 0, i;

• int loop_count = -1, timeout = 1, rr = 0, i; unsigned int seq_counter = 0, packets_out = 0, packets_in = 0, packets_loss;

• char *dst_string, *mac_string, *rr_string;
• double time_delta;

• char *dst_string, *mac_string, *rr_string, *end, *loop_interval = NULL;

• double time_delta, ping_interval = 0.0; float min = 0.0, max = 0.0, avg = 0.0, mdev = 0.0; uint8_t last_rr_cur = 0, last_rr[BATADV_RR_LEN][ETH_ALEN]; size_t packet_len; int disable_translate_mac = 0;

• struct timespec req;

  while ((optchar = getopt(argc, argv, "hc:i:t:RT")) != -1) { switch (optchar) {

@@ -86,9 +88,7 @@ static int ping(struct state *state, int argc, char **argv) ping_usage(); return EXIT_SUCCESS; case 'i':

• 	loop_interval = strtol(optarg, NULL , 10);
  

• 	if (loop_interval < 1)
  

• 		loop_interval = 1;
  

• 	loop_interval = strdup(optarg);
  

Why are you duplicating this string?

	found_args += ((*((char*)(optarg - 1)) == optchar ) ? 1 : 2);
	break;
case 't':

@@ -135,6 +135,25 @@ static int ping(struct state *state, int argc, char **argv) } }

• if (loop_interval) {

• errno = 0;
  

• ping_interval = strtod(loop_interval, &end);
  

• free(loop_interval);
  

• if (errno || end != (loop_interval + strlen(loop_interval))) {
  

Use after-free of loop_interval

• 	fprintf(stderr, "Error - invalid ping interval '%s'\n", (ULONG_MAX / 1000000 - 1.0));
  

You can't print a double as string. Please use %f, %e or %g to print a double value.

But the text actually suggest that you didn't want to print the calculated value in the first place

• 	goto out;
  

• } else if (ping_interval >= (ULONG_MAX / 1000000 - 1.0)) {
  

Similar to the previously calculated value - this doesn't make a lot of sense. You are dividing ULONG_MAX by 1e6 but we have no place where we handle something related to Msecs or usecs. tv_sec is in time_t and not related to ULONG_MAX / 1e6.

I can also not explain the -1.0 (at the moment). I first thought this was a bad attempt to floor the ping interval - but this would be on the wrong side of the comparison and there is actually a function for that. And since the ULONG_MAX / 1e6 seems to be random, I am unable to find the reason for this.

• 	fprintf(stderr, "Error - ping interval too large\n");
  

• 	goto out;
  

• } else {
  

• 	ping_interval = fmax(ping_interval, 0.001);
  

• }
  

• req.tv_sec = (long) (ping_interval);
  

time_t not long

•           req.tv_nsec = fmod(ping_interval, 1.0) * 1000000000l;
  

I think this would be a good place to use modf to split integral and fractional part.

Kind regards, Sven