Allow adding new hosts to a running noping session #20
Conversation
|
This would be extremely useful |
There was a problem hiding this comment.
Hi @hamishcoleman,
thank you very much for your patch! I think there is a small logic issue – apart from that it's mostly white space nit picking ;)
Best regards,
—octo
src/oping.c
Outdated
| @@ -293,6 +293,11 @@ static void clean_history (ping_context_t *ctx) /* {{{ */ | |||
| memcpy (ctx->history_by_value, ctx->history_by_time, | |||
| sizeof (ctx->history_by_time)); | |||
|
|
|||
| /* Remove impossible values caused by adding a new host */ | |||
| for (i = 0; i < ctx->history_size; i++) | |||
| if (ctx->history_by_value[i]<0) | |||
There was a problem hiding this comment.
Please add spaces around "<". Here and around other operators below.
src/oping.c
Outdated
| @@ -293,6 +293,11 @@ static void clean_history (ping_context_t *ctx) /* {{{ */ | |||
| memcpy (ctx->history_by_value, ctx->history_by_time, | |||
| sizeof (ctx->history_by_time)); | |||
|
|
|||
| /* Remove impossible values caused by adding a new host */ | |||
There was a problem hiding this comment.
Please make sure that all lines are intended using tabs, like the surrounding code.
src/oping.c
Outdated
| @@ -1046,6 +1068,10 @@ static int update_graph_prettyping (ping_context_t *ctx, /* {{{ */ | |||
| index = (history_offset + x) % ctx->history_size; | |||
| latency = ctx->history_by_time[index]; | |||
|
|
|||
| if (latency < 0) { | |||
There was a problem hiding this comment.
I don't think this is correct: there is an explicit case handling latency < 0 down below. This happens when no response is received, I think.
There was a problem hiding this comment.
liboping documented "less than zero" as no response received and uses a hard-coded -1.0, oping.c then converts any <0 latency into a NAN for storage in the history_by_time. This looks like it made it simpler to calculate the averages and histograms than just using the raw -1 value.
So, there is some inconsistency, but it all works (eg: new hosts get space chars for new data, missed replys get red bangs)
src/oping.c
Outdated
| @@ -1332,6 +1352,31 @@ static int check_resize (pingobj_t *ping) /* {{{ */ | |||
| else if (opt_show_graph > 0) | |||
| opt_show_graph++; | |||
| } | |||
| else if (key == 'a') | |||
| { | |||
| char host[80]; | |||
There was a problem hiding this comment.
Please use NI_MAXHOST instead of the hard-coded 80 literal.
src/oping.c
Outdated
| old data is still visible */ | ||
|
|
||
| need_resize = 1; | ||
| host_num++; |
There was a problem hiding this comment.
Please unify whitespace handling. Make sure all added lines used tabs for indentation.
* Ensure that all touched lines use tabs for indentation * Ensure operators have visible spaces around them
This also removes a "for loop initial declaration" which GCC complains about when not in C99 mode. *sigh*
|
Thanks again @hamishcoleman! |
No description provided.