From 4058461d14d359831913f1cd147579244b750f3e Mon Sep 17 00:00:00 2001 From: "Bruce A. Mah" Date: Thu, 25 Apr 2024 23:10:03 -0700 Subject: [PATCH 1/2] Shrink critical section in iperf_err(). This is similar to what was done for iperf_errexit() in a previous commit. --- src/iperf_error.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/iperf_error.c b/src/iperf_error.c index 12ab3eb7d..0fedf3110 100644 --- a/src/iperf_error.c +++ b/src/iperf_error.c @@ -47,10 +47,6 @@ iperf_err(struct iperf_test *test, const char *format, ...) struct tm *ltm = NULL; char *ct = NULL; - if (pthread_mutex_lock(&(test->print_mutex)) != 0) { - perror("iperf_err: pthread_mutex_lock"); - } - /* Timestamp if requested */ if (test != NULL && test->timestamps) { time(&now); @@ -64,6 +60,10 @@ iperf_err(struct iperf_test *test, const char *format, ...) if (test != NULL && test->json_output && test->json_top != NULL) cJSON_AddStringToObject(test->json_top, "error", str); else { + if (pthread_mutex_lock(&(test->print_mutex)) != 0) { + perror("iperf_err: pthread_mutex_lock"); + } + if (test && test->outfile && test->outfile != stdout) { if (ct) { fprintf(test->outfile, "%s", ct); @@ -76,12 +76,13 @@ iperf_err(struct iperf_test *test, const char *format, ...) } fprintf(stderr, "iperf3: %s\n", str); } - } - va_end(argp); - if (pthread_mutex_unlock(&(test->print_mutex)) != 0) { - perror("iperf_err: pthread_mutex_unlock"); + if (pthread_mutex_unlock(&(test->print_mutex)) != 0) { + perror("iperf_err: pthread_mutex_unlock"); + } + } + va_end(argp); } /* Do a printf to stderr or log file as appropriate, then exit. */ From fd96963e1e58d9933fe4666bd7071429458aac6c Mon Sep 17 00:00:00 2001 From: "Bruce A. Mah" Date: Thu, 25 Apr 2024 23:12:01 -0700 Subject: [PATCH 2/2] Restructure iperf_json_finish() to eliminate duplicate --json output. (#1688) Add locking around fprintf() calls in JSONStream_Output(). Probably not needed at the moment given that this function can only be called from the main thread, but added for consistency and possible future usage. --- src/iperf_api.c | 85 ++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/src/iperf_api.c b/src/iperf_api.c index 139ab87fb..d40561c10 100644 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -2766,13 +2766,19 @@ JSONStream_Output(struct iperf_test * test, const char * event_name, cJSON * obj { cJSON *event = cJSON_CreateObject(); if (!event) - return -1; + return -1; cJSON_AddStringToObject(event, "event", event_name); cJSON_AddItemReferenceToObject(event, "data", obj); char *str = cJSON_PrintUnformatted(event); if (str == NULL) - return -1; + return -1; + if (pthread_mutex_lock(&(test->print_mutex)) != 0) { + perror("iperf_json_finish: pthread_mutex_lock"); + } fprintf(test->outfile, "%s\n", str); + if (pthread_mutex_unlock(&(test->print_mutex)) != 0) { + perror("iperf_json_finish: pthread_mutex_unlock"); + } iflush(test); cJSON_free(str); cJSON_Delete(event); @@ -4865,49 +4871,50 @@ iperf_json_finish(struct iperf_test *test) if (test->server_output_text) { cJSON_AddStringToObject(test->json_top, "server_output_text", test->server_output_text); } - // Get ASCII rendering of JSON structure. Then make our - // own copy of it and return the storage that cJSON allocated - // on our behalf. We keep our own copy around. - char *str = cJSON_Print(test->json_top); - if (str == NULL) { - return -1; - } - test->json_output_string = strdup(str); - cJSON_free(str); - if (test->json_output_string == NULL) { - return -1; - } - if (pthread_mutex_lock(&(test->print_mutex)) != 0) { - perror("iperf_json_finish: pthread_mutex_lock"); + /* --json-stream, so we print various individual objects */ + if (test->json_stream) { + cJSON *error = cJSON_GetObjectItem(test->json_top, "error"); + if (error) { + JSONStream_Output(test, "error", error); + } + if (test->json_server_output) { + JSONStream_Output(test, "server_output_json", test->json_server_output); + } + if (test->server_output_text) { + JSONStream_Output(test, "server_output_text", cJSON_CreateString(test->server_output_text)); + } + JSONStream_Output(test, "end", test->json_end); } - fprintf(test->outfile, "%s\n", test->json_output_string); - if (pthread_mutex_unlock(&(test->print_mutex)) != 0) { - perror("iperf_json_finish: pthread_mutex_unlock"); + /* Original --json output, single monolithic object */ + else { + /* + * Get ASCII rendering of JSON structure. Then make our + * own copy of it and return the storage that cJSON + * allocated on our behalf. We keep our own copy + * around. + */ + char *str = cJSON_Print(test->json_top); + if (str == NULL) { + return -1; + } + test->json_output_string = strdup(str); + cJSON_free(str); + if (test->json_output_string == NULL) { + return -1; + } + if (pthread_mutex_lock(&(test->print_mutex)) != 0) { + perror("iperf_json_finish: pthread_mutex_lock"); + } + fprintf(test->outfile, "%s\n", test->json_output_string); + if (pthread_mutex_unlock(&(test->print_mutex)) != 0) { + perror("iperf_json_finish: pthread_mutex_unlock"); + } + iflush(test); } - iflush(test); cJSON_Delete(test->json_top); - test->json_top = NULL; } - if (test->json_stream) { - cJSON *error = cJSON_GetObjectItem(test->json_top, "error"); - if (error) { - JSONStream_Output(test, "error", error); - } - if (test->json_server_output) { - JSONStream_Output(test, "server_output_json", test->json_server_output); - } - if (test->server_output_text) { - JSONStream_Output(test, "server_output_text", cJSON_CreateString(test->server_output_text)); - } - JSONStream_Output(test, "end", test->json_end); - } - else { - fprintf(test->outfile, "%s\n", test->json_output_string); - iflush(test); - } - cJSON_Delete(test->json_top); test->json_top = test->json_start = test->json_connected = test->json_intervals = test->json_server_output = test->json_end = NULL; return 0; }