[lttng-dev] [PATCH babeltrace] Fix: Replace bt_timegm with a thread-safe implementation
Jérémie Galarneau
jeremie.galarneau at efficios.com
Thu Nov 23 19:02:15 UTC 2017
Merged, thanks!
Jérémie
On 13 November 2017 at 15:17, Michael Jeanson <mjeanson at efficios.com> wrote:
> The current compat wrapper for timegm is not thread-safe, it modifies
> the process wide time settings with tzset. This was not a problem in bt1
> as it was only used in the cli binary but it is now used in the trimmer
> plugin which is used by the library.
>
> Replace the wrapper with a basic implementation and add more tests.
>
> Signed-off-by: Michael Jeanson <mjeanson at efficios.com>
> ---
> include/babeltrace/compat/utc-internal.h | 76 +++++++++-------
> tests/cli/test_trimmer.in | 145 ++++++++++++++++++++++++++++---
> 2 files changed, 177 insertions(+), 44 deletions(-)
>
> diff --git a/include/babeltrace/compat/utc-internal.h b/include/babeltrace/compat/utc-internal.h
> index db3035a..da19db8 100644
> --- a/include/babeltrace/compat/utc-internal.h
> +++ b/include/babeltrace/compat/utc-internal.h
> @@ -44,50 +44,60 @@ time_t bt_timegm(struct tm *tm)
>
> #else
>
> -#include <string.h>
> -#include <stdlib.h>
> -#include <glib.h>
> +#include <errno.h>
>
> /*
> - * Note: Below implementation of timegm() is not thread safe
> - * as it changes the environment
> - * variable TZ. It is OK as long as it is kept in self-contained program,
> - * but should not be used within thread-safe library code.
> + * This is a simple implementation of timegm() it just turns the "struct tm" into
> + * a GMT time_t. It does not normalize any of the fields of the "struct tm", nor
> + * does it set tm_wday or tm_yday.
> */
>
> static inline
> +int bt_leapyear(int year)
> +{
> + return ((year % 4) == 0 && ((year % 100) != 0 || (year % 400) == 0));
> +}
> +
> +static inline
> time_t bt_timegm(struct tm *tm)
> {
> - time_t ret;
> - char *tz;
> -
> - tz = getenv("TZ");
> - /*
> - * Make a temporary copy, as the environment variable will be
> - * modified.
> - */
> - if (tz) {
> - tz = strdup(tz);
> - if (!tz) {
> - /*
> - * Memory allocation error.
> - */
> - return (time_t) -1;
> + int year, month, total_days;
> +
> + int monthlen[2][12] = {
> + /* Days per month for a regular year */
> + { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 },
> + /* Days per month for a leap year */
> + { 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 },
> + };
> +
> + if ((tm->tm_mon >= 12) ||
> + (tm->tm_mday >= 32) ||
> + (tm->tm_hour >= 24) ||
> + (tm->tm_min >= 60) ||
> + (tm->tm_sec >= 61)) {
> + errno = EOVERFLOW;
> + return (time_t) -1;
> + }
> +
> + /* Add 365 days for each year since 1970 */
> + total_days = 365 * (tm->tm_year - 70);
> +
> + /* Add one day for each leap year since 1970 */
> + for (year = 70; year < tm->tm_year; year++) {
> + if (bt_leapyear(1900 + year)) {
> + total_days++;
> }
> }
>
> - /* Temporarily setting TZ to 1 for UTC */
> - g_setenv("TZ", "", 1);
> - tzset();
> - ret = mktime(tm);
> - if (tz) {
> - g_setenv("TZ", tz, 1);
> - free(tz);
> - } else {
> - unsetenv("TZ");
> + /* Add days for each remaining month */
> + for (month = 0; month < tm->tm_mon; month++) {
> + total_days += monthlen[bt_leapyear(1900 + year)][month];
> }
> - tzset();
> - return ret;
> +
> + /* Add remaining days */
> + total_days += tm->tm_mday - 1;
> +
> + return ((((total_days * 24) + tm->tm_hour) * 60 + tm->tm_min) * 60 + tm->tm_sec);
> }
>
> #endif
> diff --git a/tests/cli/test_trimmer.in b/tests/cli/test_trimmer.in
> index e69d7e0..249fffa 100644
> --- a/tests/cli/test_trimmer.in
> +++ b/tests/cli/test_trimmer.in
> @@ -19,7 +19,7 @@
>
> TRACE_PATH="${BT_CTF_TRACES}/succeed/wk-heartbeat-u/"
>
> -NUM_TESTS=10
> +NUM_TESTS=44
>
> plan_tests $NUM_TESTS
>
> @@ -27,40 +27,163 @@ tmp_out=$(mktemp)
>
> "${BT_BIN}" --clock-gmt --begin 17:48:17.587029529 --end 17:48:17.588680018 \
> "${TRACE_PATH}" >/dev/null 2>&1
> -ok $? "Read the trace with the trimmer enabled"
> +ok $? "Read a trace with the trimmer enabled (GMT relative timestamps)"
>
> "${BT_BIN}" --clock-gmt --begin 17:48:17.587029529 "${TRACE_PATH}" \
> 2>/dev/null >"${tmp_out}"
> -ok $? "Running with --begin"
> +ok $? "Ran successfully with --begin (GMT relative timestamps)"
> cnt=$(wc -l < "${tmp_out}")
> test $cnt == 18
> -ok $? "Expected number of events after trimming begin and end"
> +ok $? "Received ${cnt}/18 events (GMT relative timestamps)"
>
> "${BT_BIN}" --clock-gmt --end 17:48:17.588680018 "${TRACE_PATH}" \
> 2>/dev/null >"${tmp_out}"
> -ok $? "Running with --end"
> +ok $? "Ran successfully with --end (GMT relative timestamps)"
> cnt=$(wc -l < "${tmp_out}")
> test $cnt == 9
> -ok $? "Expected number of events after trimming end"
> +ok $? "Received ${cnt}/9 events (GMT relative timestamps)"
>
> "${BT_BIN}" --clock-gmt --begin 17:48:17.587029529 --end 17:48:17.588680018 \
> "${TRACE_PATH}" 2>/dev/null >"${tmp_out}"
> -ok $? "Running with --begin and --end"
> +ok $? "Ran successfully with --begin and --end (GMT relative timestamps)"
> cnt=$(wc -l < "${tmp_out}")
> test $cnt == 7
> -ok $? "Expected number of events after trimming begin and end"
> +ok $? "Received ${cnt}/7 events (GMT relative timestamps)"
>
> "${BT_BIN}" --clock-gmt --begin 18:48:17.587029529 "${TRACE_PATH}" \
> 2>/dev/null >"${tmp_out}"
> -ok $? "Running with --begin out of range"
> +ok $? "Ran successfully with --begin out of range (GMT relative timestamps)"
> cnt=$(wc -l < "${tmp_out}")
> test $cnt == 0
> -ok $? "No event output when begin is after the end of the trace"
> +ok $? "No events when begin is out of range (GMT relative timestamps)"
>
> "${BT_BIN}" --clock-gmt --end 16:48:17.588680018 "${TRACE_PATH}" \
> 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end out of range (GMT relative timestamps)"
> cnt=$(wc -l < "${tmp_out}")
> test $cnt == 0
> -ok $? "No event output when end is before the beginning of the trace"
> +ok $? "No events when end is out of range (GMT relative timestamps)"
> +
> +
> +"${BT_BIN}" --clock-gmt --begin "2012-10-29 17:48:17.587029529" --end "2012-10-29 17:48:17.588680018" \
> + "${TRACE_PATH}" >/dev/null 2>&1
> +ok $? "Read a trace with the trimmer enabled (GMT absolute timestamps)"
> +
> +"${BT_BIN}" --clock-gmt --begin "2012-10-29 17:48:17.587029529" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin (GMT absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 18
> +ok $? "Received ${cnt}/18 events (GMT absolute timestamps)"
> +
> +"${BT_BIN}" --clock-gmt --end "2012-10-29 17:48:17.588680018" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end (GMT absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 9
> +ok $? "Received ${cnt}/9 events (GMT absolute timestamps)"
> +
> +"${BT_BIN}" --clock-gmt --begin "2012-10-29 17:48:17.587029529" --end "2012-10-29 17:48:17.588680018" \
> + "${TRACE_PATH}" 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin and --end (GMT absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 7
> +ok $? "Received ${cnt}/7 events (GMT absolute timestamps)"
> +
> +"${BT_BIN}" --clock-gmt --begin "2012-10-29 18:48:17.587029529" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin out of range (GMT absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 0
> +ok $? "No events when begin is out of range (GMT absolute timestamps)"
> +
> +"${BT_BIN}" --clock-gmt --end "2012-10-29 16:48:17.588680018" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end out of range (GMT absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 0
> +ok $? "No events when end is out of range (GMT absolute timestamps)"
> +
> +
> +export TZ=EST
> +
> +"${BT_BIN}" --begin "12:48:17.587029529" --end "12:48:17.588680018" \
> + "${TRACE_PATH}" >/dev/null 2>&1
> +ok $? "Read a trace with the trimmer enabled (EST relative timestamps)"
> +
> +"${BT_BIN}" --begin "12:48:17.587029529" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin (EST relative timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 18
> +ok $? "Received ${cnt}/18 events (EST relative timestamps)"
> +
> +"${BT_BIN}" --end "12:48:17.588680018" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end (EST relative timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 9
> +ok $? "Received ${cnt}/9 events (EST relative timestamps)"
> +
> +"${BT_BIN}" --begin "12:48:17.587029529" --end "12:48:17.588680018" \
> + "${TRACE_PATH}" 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin and --end (EST relative timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 7
> +ok $? "Received ${cnt}/7 events (EST relative timestamps)"
> +
> +"${BT_BIN}" --begin "13:48:17.587029529" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin out of range (EST relative timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 0
> +ok $? "No events when begin is out of range (EST relative timestamps)"
> +
> +"${BT_BIN}" --end "11:48:17.588680018" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end out of range (EST relative timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 0
> +ok $? "No events when end is out of range (EST relative timestamps)"
> +
> +
> +"${BT_BIN}" --begin "2012-10-29 12:48:17.587029529" --end "2012-10-29 12:48:17.588680018" \
> + "${TRACE_PATH}" >/dev/null 2>&1
> +ok $? "Read a trace with the trimmer enabled (EST absolute timestamps)"
> +
> +"${BT_BIN}" --begin "2012-10-29 12:48:17.587029529" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin (EST absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 18
> +ok $? "Received ${cnt}/18 events (EST absolute timestamps)"
> +
> +"${BT_BIN}" --end "2012-10-29 12:48:17.588680018" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end (EST absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 9
> +ok $? "Received ${cnt}/9 events (EST absolute timestamps)"
> +
> +"${BT_BIN}" --begin "2012-10-29 12:48:17.587029529" --end "2012-10-29 12:48:17.588680018" \
> + "${TRACE_PATH}" 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin and --end (EST absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 7
> +ok $? "Received ${cnt}/7 events (EST absolute timestamps)"
> +
> +"${BT_BIN}" --begin "2012-10-29 13:48:17.587029529" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --begin out of range (EST absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 0
> +ok $? "No events when begin is out of range (EST absolute timestamps)"
> +
> +"${BT_BIN}" --end "2012-10-29 11:48:17.588680018" "${TRACE_PATH}" \
> + 2>/dev/null >"${tmp_out}"
> +ok $? "Ran successfully with --end out of range (EST absolute timestamps)"
> +cnt=$(wc -l < "${tmp_out}")
> +test $cnt == 0
> +ok $? "No events when end is out of range (EST absolute timestamps)"
>
> rm "${tmp_out}"
> --
> 2.7.4
>
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list