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 <[email protected]> > --- > 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
signature.asc
Description: This is a digitally signed message part.
