From 8ea5a8a2e154a1459b986629694c850234b2f659 Mon Sep 17 00:00:00 2001 From: "E. Madison Bray" <676149+embray@users.noreply.github.com> Date: Thu, 26 Mar 2026 11:51:48 +0100 Subject: [PATCH 1/5] feat: add asdf/core/time-1.4.0 extension Implement the asdf/core/time-1.4.0 schema as an extension, supporting deserialization and serialization of time values from/to ASDF files. The extension handles these time formats: ISO 8601, ordinal (yday), Unix epoch, Julian Date, Modified Julian Date, and Besselian year, with optional time scale and observer location. --- Makefile.am | 1 + include/CMakeLists.txt | 1 + include/Makefile.am | 1 + include/asdf/core/asdf.h | 1 + include/asdf/core/history_entry.h | 8 +- include/asdf/core/time.h | 86 +++++ src/CMakeLists.txt | 1 + src/core/history_entry.c | 137 ++----- src/core/time.c | 583 ++++++++++++++++++++++++++++++ 9 files changed, 712 insertions(+), 107 deletions(-) create mode 100644 include/asdf/core/time.h create mode 100644 src/core/time.c diff --git a/Makefile.am b/Makefile.am index aee4d755..74103382 100644 --- a/Makefile.am +++ b/Makefile.am @@ -13,6 +13,7 @@ src_files = \ src/core/ndarray.c \ src/core/ndarray_convert.c \ src/core/software.c \ + src/core/time.c \ src/emitter.c \ src/error.c \ src/event.c \ diff --git a/include/CMakeLists.txt b/include/CMakeLists.txt index 846b1372..89cb47f2 100644 --- a/include/CMakeLists.txt +++ b/include/CMakeLists.txt @@ -28,6 +28,7 @@ install( asdf/core/extension_metadata.h asdf/core/software.h asdf/core/ndarray.h + asdf/core/time.h DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/asdf/core" ) diff --git a/include/Makefile.am b/include/Makefile.am index 9f23ad23..48538771 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -6,6 +6,7 @@ nobase_include_HEADERS = \ asdf/core/history_entry.h \ asdf/core/ndarray.h \ asdf/core/software.h \ + asdf/core/time.h \ asdf/emitter.h \ asdf/error.h \ asdf/event.h \ diff --git a/include/asdf/core/asdf.h b/include/asdf/core/asdf.h index cf1d444a..0a365522 100644 --- a/include/asdf/core/asdf.h +++ b/include/asdf/core/asdf.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include diff --git a/include/asdf/core/history_entry.h b/include/asdf/core/history_entry.h index 253a0f2a..c168ace9 100644 --- a/include/asdf/core/history_entry.h +++ b/include/asdf/core/history_entry.h @@ -8,12 +8,14 @@ #include #include +#include + ASDF_BEGIN_DECLS typedef struct { const char *description; - struct timespec time; + const asdf_time_t *time; const asdf_software_t **software; } asdf_history_entry_t; @@ -28,8 +30,8 @@ ASDF_DECLARE_EXTENSION(history_entry, asdf_history_entry_t); * Allow passing a timestamp for the history entry as well, or an extended * version that accepts a timestamp. * - * :params file: Open `asdf_file_t *` handle - * :params description: The text to add to the history entry + * :param file: Open `asdf_file_t *` handle + * :param description: The text to add to the history entry * :return: Non-zero if adding the history entry failed */ ASDF_EXPORT int asdf_history_entry_add(asdf_file_t *file, const char *description); diff --git a/include/asdf/core/time.h b/include/asdf/core/time.h new file mode 100644 index 00000000..41f41c56 --- /dev/null +++ b/include/asdf/core/time.h @@ -0,0 +1,86 @@ +/** Data type and extension for the stsci.edu/schemas/asdf/time/time schema */ +#ifndef ASDF_CORE_TIME_H +#define ASDF_CORE_TIME_H + +#include +#include + + +ASDF_BEGIN_DECLS + +#define ASDF_CORE_TIME_TAG "tag:stsci.edu:asdf/time/time-1.4.0" +#define ASDF_TIME_TIMESTR_MAXLEN 255 + +typedef enum { + ASDF_TIME_FORMAT_ISO_TIME = 0, + ASDF_TIME_FORMAT_YDAY, + ASDF_TIME_FORMAT_BYEAR, + ASDF_TIME_FORMAT_JYEAR, + ASDF_TIME_FORMAT_DECIMALYEAR, + ASDF_TIME_FORMAT_JD, + ASDF_TIME_FORMAT_MJD, + ASDF_TIME_FORMAT_GPS, + ASDF_TIME_FORMAT_UNIX, + ASDF_TIME_FORMAT_UTIME, + ASDF_TIME_FORMAT_TAI_SECONDS, + ASDF_TIME_FORMAT_CXCSEC, + ASDF_TIME_FORMAT_GALEXSEC, + ASDF_TIME_FORMAT_UNIX_TAI, + ASDF_TIME_FORMAT_RESERVED1, + /* "other" format(s) below */ + ASDF_TIME_FORMAT_BYEAR_STR, + ASDF_TIME_FORMAT_DATETIME, + ASDF_TIME_FORMAT_FITS, + ASDF_TIME_FORMAT_ISOT, + ASDF_TIME_FORMAT_JYEAR_STR, + ASDF_TIME_FORMAT_PLOT_DATE, + ASDF_TIME_FORMAT_YMDHMS, + ASDF_TIME_FORMAT_datetime64, +} asdf_time_base_format_t; + + +typedef enum { + ASDF_TIME_SCALE_UTC = 0, + ASDF_TIME_SCALE_TAI, + ASDF_TIME_SCALE_TCB, + ASDF_TIME_SCALE_TCG, + ASDF_TIME_SCALE_TDB, + ASDF_TIME_SCALE_TT, + ASDF_TIME_SCALE_UT1, +} asdf_time_scale_t; + +typedef struct { + double longitude; + double latitude; + double height; +} asdf_time_location_t; + +typedef struct { + bool is_base_format; + asdf_time_base_format_t type; +} asdf_time_format_t; + +struct asdf_time_info_t { + struct timespec ts; + struct tm tm; +}; + +typedef struct { + char *value; + struct asdf_time_info_t info; + asdf_time_format_t format; + asdf_time_scale_t scale; + asdf_time_location_t location; +} asdf_time_t; + +ASDF_DECLARE_EXTENSION(time, asdf_time_t); + +ASDF_LOCAL int asdf_time_parse_std( + const char *s, const asdf_time_format_t *format, struct asdf_time_info_t *out); +ASDF_LOCAL int asdf_time_parse_byear(const char *s, struct asdf_time_info_t *out); +ASDF_LOCAL int asdf_time_parse_yday(const char *s, struct asdf_time_info_t *out); + + +ASDF_END_DECLS + +#endif /* ASDF_CORE_TIME_H */ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1b79d583..4ce7057f 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -13,6 +13,7 @@ set(libasdf_sources core/ndarray.c core/ndarray_convert.c core/software.c + core/time.c block.c context.c error.c diff --git a/src/core/history_entry.c b/src/core/history_entry.c index a6eed311..932df8ab 100644 --- a/src/core/history_entry.c +++ b/src/core/history_entry.c @@ -2,10 +2,8 @@ #include "config.h" #endif -#include #include #include -#include #include "../error.h" #include "../extension_util.h" @@ -17,98 +15,11 @@ #include "asdf.h" #include "history_entry.h" #include "software.h" +#include "time.h" #define ASDF_CORE_HISTORY_ENTRY_TAG ASDF_CORE_TAG_PREFIX "history_entry-1.0.0" -/* - * Parse a YAML-serialized timestamp - * - * Generally in ISO8601 but can be "relaxed" having a space between the date and the time (the - * Python asdf actually appears to output in this format though maybe it depends on the Python - * yaml version--we should specify this more strictly maybe... - */ -#ifdef HAVE_STRPTIME -#define NSEC_PER_SEC 1e9 // In case this ever changes -#define SEC_PER_HOUR 3600 -#define SEC_PER_MIN 60 - -static int asdf_parse_datetime(const char *scalar, struct timespec *out) { - if (!scalar || !out) - return -1; - - struct tm tm = {0}; - char tz_sign = 0; - int tz_hour = 0; - int tz_min = 0; - long nsec = 0; - bool has_time = false; - char *rest = NULL; - char *buf = strdup(scalar); - - if (!buf) - return -1; - - // Normalize separators (replace 'T' or 't' with space) - for (char *chr = buf; *chr; ++chr) - if (*chr == 'T' || *chr == 't') - *chr = ' '; - - // Try to parse date and time (without optional fractional seconds and timezone) - rest = strptime(buf, "%Y-%m-%d %H:%M:%S", &tm); - - if (!rest) - rest = strptime(buf, "%Y-%m-%d", &tm); - else - has_time = true; - - if (!rest) { - free(buf); - return -1; - } - - // Handle optional fractional seconds - if (has_time) { - const char *dot = strchr(rest, '.'); - if (dot) { - double frac = 0; - sscanf(dot, "%lf", &frac); - nsec = (long)((frac - (int)frac) * NSEC_PER_SEC); - } - - // Handle timezone offsets (Z/z = Zulu is ignored, just don't add any offset) - const char *tz = strpbrk(rest, "+-"); - if (tz && (*tz == '+' || *tz == '-')) { - tz_sign = (*tz == '-') ? -1 : 1; - if (sscanf(tz + 1, "%2d:%2d", &tz_hour, &tz_min) < 1) - sscanf(tz + 1, "%2d", &tz_hour); - } - } - - // Convert to time_t and adjust for time zone - time_t time = timegm(&tm); - if (time == (time_t)-1) { - free(buf); - return -1; - } - - time -= (long)tz_sign * (tz_hour * SEC_PER_HOUR + tz_min * SEC_PER_MIN); - out->tv_sec = time; - out->tv_nsec = nsec; - free(buf); - return 0; -} -#else -#warning "strptime() not available, times will not be parsed" -static int asdf_parse_datetime(UNUSED(const char *s), struct timespec *out) { - if (out) { - out->tv_sec = 0; - out->tv_nsec = 0; - } - return 0; -} -#endif - static asdf_value_t *asdf_history_entry_serialize( asdf_file_t *file, @@ -237,14 +148,13 @@ static asdf_software_t **asdf_history_entry_deserialize_software(asdf_value_t *v return software; } - static asdf_value_err_t asdf_history_entry_deserialize( asdf_value_t *value, UNUSED(const void *userdata), void **out) { asdf_value_err_t err = ASDF_VALUE_ERR_PARSE_FAILURE; asdf_value_t *prop = NULL; const char *description = NULL; const char *time_str = NULL; - struct timespec time = {0}; + asdf_time_t *time = NULL; asdf_software_t **software = NULL; asdf_mapping_t *entry_map = NULL; @@ -264,24 +174,33 @@ static asdf_value_err_t asdf_history_entry_deserialize( prop = asdf_mapping_get(entry_map, "time"); if (prop) { - bool valid_time = false; - if (ASDF_VALUE_OK == asdf_value_as_string0(prop, &time_str)) { - if (0 == asdf_parse_datetime(time_str, &time)) + + // cast the value of "time" to an asdf_time_t + const asdf_extension_t *time_ext = asdf_extension_get(value->file, ASDF_CORE_TIME_TAG); + if (time_ext) { + bool valid_time = false; + time_ext->deserialize(prop, NULL, (void *)&time); + + if (time) { valid_time = true; - } + } #ifdef ASDF_LOG_ENABLED - if (!valid_time) { - if (ASDF_VALUE_OK != asdf_value_as_scalar0(prop, &time_str)) { - time_str = ""; + if (!valid_time) { + if (ASDF_VALUE_OK != asdf_value_as_scalar0(prop, &time_str)) { + time_str = ""; + } + ASDF_LOG( + value->file, + ASDF_LOG_WARN, + "ignoring invalid time %s in history_entry", + time_str); } - ASDF_LOG( - value->file, ASDF_LOG_WARN, "ignoring invalid time %s in history_entry", time_str); - } #endif + } + asdf_value_destroy(prop); } - asdf_value_destroy(prop); /* Software can be either an array of software or a single entry, but here it is always * returned as a NULL-terminated array of asdf_software_t * @@ -305,6 +224,7 @@ static asdf_value_err_t asdf_history_entry_deserialize( entry->time = time; entry->software = (const asdf_software_t **)software; *out = entry; + return ASDF_VALUE_OK; failure: asdf_value_destroy(prop); @@ -321,6 +241,10 @@ static void asdf_history_entry_dealloc(void *value) { free((void *)entry->description); + if (entry->time) { + asdf_time_destroy((asdf_time_t *)entry->time); + } + if (entry->software) { for (asdf_software_t **sp = (asdf_software_t **)entry->software; *sp; ++sp) { asdf_software_destroy(*sp); @@ -350,7 +274,12 @@ static void *asdf_history_entry_copy(const void *value) { goto failure; } - copy->time = entry->time; + if (entry->time) { + copy->time = asdf_time_clone(entry->time); + + if (!copy->time) + goto failure; + } if (entry->software) { copy->software = (const asdf_software_t **)asdf_software_array_clone(entry->software); diff --git a/src/core/time.c b/src/core/time.c new file mode 100644 index 00000000..512d619d --- /dev/null +++ b/src/core/time.c @@ -0,0 +1,583 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include +#include + +#include "asdf.h" +#include "time.h" + +#include "../extension_registry.h" +#include "../log.h" +#include "../util.h" +#include "../value.h" +#include "stc/cregex.h" + +#ifdef HAVE_STRPTIME +static const char *ASDF_TIME_SFMT_ISO_TIME[] = {"%Y-%m-%d %H:%M:%S", "%Y-%m-%d"}; +static const char *ASDF_TIME_SFMT_YDAY[] = {"%Y:%j:%H:%M:%S", "%Y:%j"}; +static const char *ASDF_TIME_SFMT_UNIX[] = {"%s"}; + +#define check_format_strptime(TYPE, BUF, TM, HAS_TIME, STATUS) \ + { \ + size_t idx = 0; \ + do { \ + (STATUS) = strptime((BUF), (TYPE)[idx], (TM)); \ + if ((STATUS)) { \ + (HAS_TIME) = true; \ + break; \ + } \ + } while (idx++ && idx < sizeof((TYPE)) / sizeof(*(TYPE))); \ + } + +#define JD_B1900 2415020.31352 +#define JD_MJD 2400000.5 + +/* Calendar constants */ +static const double JD_GREGORIAN_START = 2299161.0; +static const double JD_CORRECTION_REF = 1867216.25; +static const double JD_CALENDAR_OFFSET = 122.1; +static const int JD_BASE_YEAR = 4716; +static const double JD_YEAR_LENGTH = 365.25; +static const double GREGORIAN_OFFSET = (int)1524.0; + +static const double AVG_MONTH_LENGTH = 30.6001; +static const double AVG_YEAR_LENGTH = 365.242198781; +static const double DAYS_IN_CENTURY = 36524.2198781; +static const int SECONDS_PER_DAY = 86400; +static const int SECONDS_PER_HOUR = 3600; +static const int SECONDS_PER_MINUTE = 60; + + +/* Julian Date to Gregorian calendar conversion */ +static void julian_to_tm(const double jd, struct tm *t, time_t *nanoseconds) { + const double jd_shift = jd + 0.5; + const int jd_int = (int)jd_shift; + const double day_fraction = jd_shift - jd_int; + + int jd_adjust; + if (jd_int < JD_GREGORIAN_START) { + jd_adjust = jd_int; + } else { + const int leap_adjust = (int)((jd_int - JD_CORRECTION_REF) / DAYS_IN_CENTURY); + jd_adjust = jd_int + 1 + leap_adjust - leap_adjust / 4; + } + + const int calendar_day = jd_adjust + GREGORIAN_OFFSET; + const int year_base = (int)((calendar_day - JD_CALENDAR_OFFSET) / JD_YEAR_LENGTH); + const int days_in_years = (int)(JD_YEAR_LENGTH * year_base); + const int month_base = (int)((calendar_day - days_in_years) / AVG_MONTH_LENGTH); + + const int day = calendar_day - days_in_years - (int)(AVG_MONTH_LENGTH * month_base) + + day_fraction; + const int month = month_base < 14 ? month_base - 1 : month_base - 13; + const int year = month > 2 ? year_base - JD_BASE_YEAR : year_base - JD_BASE_YEAR - 1; + + const double total_seconds = day_fraction * SECONDS_PER_DAY + 0.5; + const int hour = (int)(total_seconds / SECONDS_PER_HOUR); + const int minute = (int)((total_seconds - hour * SECONDS_PER_HOUR) / SECONDS_PER_MINUTE); + const double seconds_whole = total_seconds - hour * SECONDS_PER_HOUR - + minute * SECONDS_PER_MINUTE; + const int second = (int)seconds_whole; + const double fractional_seconds = seconds_whole - second; + + t->tm_year = year - 1900; + t->tm_mon = month - 1; + t->tm_mday = day; + t->tm_hour = hour; + t->tm_min = minute; + t->tm_sec = second; + + if (nanoseconds) { + *nanoseconds = (time_t)(fractional_seconds * 1e9) + 0.5; + } +} + + +static void mjd_to_tm(const double mjd, struct tm *t, time_t *nsec) { + const double jd = mjd + JD_MJD; + julian_to_tm(jd, t, nsec); +} + + +static double besselian_to_julian(const double b) { + return JD_B1900 + AVG_YEAR_LENGTH * (b - 1900.0); +} + + +int asdf_time_parse_std( + const char *s, const asdf_time_format_t *format, struct asdf_time_info_t *out) { + if (!s || !out) { + return -1; + } + + struct tm tm = {0}; + char tz_sign = 0; + int tz_hour = 0; + int tz_min = 0; + long nsec = 0; + bool has_time = false; + char *rest = NULL; + char *buf = strdup(s); + + if (!buf) { + return -1; + } + + /* Normalize separators (replace 'T' or 't' with space) */ + for (char *c = buf; *c; ++c) { + if (*c == 'T' || *c == 't') + *c = ' '; + } + + switch (format->type) { + case ASDF_TIME_FORMAT_DATETIME: + case ASDF_TIME_FORMAT_ISO_TIME: + check_format_strptime(ASDF_TIME_SFMT_ISO_TIME, buf, &tm, has_time, rest); + break; + case ASDF_TIME_FORMAT_YDAY: + check_format_strptime(ASDF_TIME_SFMT_YDAY, buf, &tm, has_time, rest); + break; + case ASDF_TIME_FORMAT_UNIX: + check_format_strptime(ASDF_TIME_SFMT_UNIX, buf, &tm, has_time, rest); + break; + default: + free(buf); + return -1; + } + + if (!rest) { + free(buf); + return -1; + } + + /* Handle optional fractional seconds */ + if (has_time) { + const char *dot = strchr(rest, '.'); + if (dot) { + double frac = 0; + sscanf(dot, "%lf", &frac); + nsec = (long)((frac - (int)frac) * 1e9); + } + + /* Handle timezone offsets (Z/z = Zulu is ignored, just don't add any offset) */ + const char *tz = strpbrk(rest, "+-"); + if (tz && (*tz == '+' || *tz == '-')) { + tz_sign = *tz == '-' ? -1 : 1; + if (sscanf(tz + 1, "%2d:%2d", &tz_hour, &tz_min) < 1) + sscanf(tz + 1, "%2d", &tz_hour); + } + } + + /* Convert to time_t and adjust for time zone */ + time_t t = timegm(&tm); + if (t == (time_t)-1) { + free(buf); + return -1; + } + + t -= tz_sign * (tz_hour * SECONDS_PER_HOUR + tz_min * SECONDS_PER_MINUTE); + + out->tm = *gmtime(&t); + out->ts.tv_sec = t; + out->ts.tv_nsec = nsec; + free(buf); + return 0; +} + + +static int asdf_time_parse_jd(const char *s, struct asdf_time_info_t *out) { + const double jd = strtod(s, NULL); + struct tm jd_tm; + time_t nsec = 0; + julian_to_tm(jd, &jd_tm, &nsec); + const time_t t = timegm(&jd_tm); + + if (out) { + out->tm = jd_tm; + out->ts.tv_sec = t; + out->ts.tv_nsec = nsec; + } else { + return -1; + } + return 0; +} + + +static int asdf_time_parse_mjd(const char *s, struct asdf_time_info_t *out) { + const double mjd = strtod(s, NULL); + struct tm mjd_tm; + time_t nsec = 0; + mjd_to_tm(mjd, &mjd_tm, &nsec); + const time_t t = timegm(&mjd_tm); + + if (out) { + out->tm = mjd_tm; + out->ts.tv_sec = t; + out->ts.tv_nsec = nsec; + } else { + return -1; + } + return 0; +} + + +int asdf_time_parse_byear(const char *s, struct asdf_time_info_t *out) { + const double byear = strtod(s, NULL); + const double jd = besselian_to_julian(byear); + struct tm tm; + time_t nsec = 0; + + julian_to_tm(jd, &tm, &nsec); + const time_t t = timegm(&tm); + + if (out) { + out->tm = *gmtime(&t); + out->ts.tv_sec = t; + out->ts.tv_nsec = nsec; + } else { + return -1; + } + return 0; +} + + +int asdf_time_parse_yday(const char *s, struct asdf_time_info_t *out) { + const asdf_time_format_t fmt = {.is_base_format = true, .type = ASDF_TIME_FORMAT_YDAY}; + return asdf_time_parse_std(s, &fmt, out); +} + +#else +#warning "strptime() not available, times will not be parsed" +static int asdf_time_parse_std(UNUSED(const char *s), struct timespec *out) { + if (out) { + out->tv_sec = 0; + out->tv_nsec = 0; + } + return 0; +} +#endif + + +static int asdf_time_parse_time( + const char *s, const asdf_time_format_t *format, struct asdf_time_info_t *out) { + int status = -1; + switch (format->type) { + case ASDF_TIME_FORMAT_YDAY: + case ASDF_TIME_FORMAT_ISO_TIME: + case ASDF_TIME_FORMAT_DATETIME: + case ASDF_TIME_FORMAT_UNIX: + status = asdf_time_parse_std(s, format, out); + break; + case ASDF_TIME_FORMAT_MJD: + status = asdf_time_parse_mjd(s, out); + break; + case ASDF_TIME_FORMAT_JD: + status = asdf_time_parse_jd(s, out); + break; + case ASDF_TIME_FORMAT_BYEAR: + status = asdf_time_parse_byear(s, out); + break; + default: + break; + } + return status; +} + + +/* + * Lookup table: asdf_time_base_format_t enum value -> YAML format name string + * + * Entries must be kept in the same order as asdf_time_base_format_t. + */ +static const char *const asdf_time_format_names[] = { + "iso_time", /* ASDF_TIME_FORMAT_ISO_TIME */ + "yday", /* ASDF_TIME_FORMAT_YDAY */ + "byear", /* ASDF_TIME_FORMAT_BYEAR */ + "jyear", /* ASDF_TIME_FORMAT_JYEAR */ + "decimalyear", /* ASDF_TIME_FORMAT_DECIMALYEAR */ + "jd", /* ASDF_TIME_FORMAT_JD */ + "mjd", /* ASDF_TIME_FORMAT_MJD */ + "gps", /* ASDF_TIME_FORMAT_GPS */ + "unix", /* ASDF_TIME_FORMAT_UNIX */ + "utime", /* ASDF_TIME_FORMAT_UTIME */ + "tai_seconds", /* ASDF_TIME_FORMAT_TAI_SECONDS */ + "cxcsec", /* ASDF_TIME_FORMAT_CXCSEC */ + "galexsec", /* ASDF_TIME_FORMAT_GALEXSEC */ + "unix_tai", /* ASDF_TIME_FORMAT_UNIX_TAI */ + NULL, /* ASDF_TIME_FORMAT_RESERVED1 */ + "byear_str", /* ASDF_TIME_FORMAT_BYEAR_STR */ + "datetime", /* ASDF_TIME_FORMAT_DATETIME */ + "fits", /* ASDF_TIME_FORMAT_FITS */ + "isot", /* ASDF_TIME_FORMAT_ISOT */ + "jyear_str", /* ASDF_TIME_FORMAT_JYEAR_STR */ + "plot_date", /* ASDF_TIME_FORMAT_PLOT_DATE */ + "ymdhms", /* ASDF_TIME_FORMAT_YMDHMS */ + "datetime64", /* ASDF_TIME_FORMAT_datetime64 */ +}; + + +/* + * Lookup table: asdf_time_scale_t enum value -> YAML scale name string + */ +static const char *const asdf_time_scale_names[] = { + "utc", /* ASDF_TIME_SCALE_UTC */ + "tai", /* ASDF_TIME_SCALE_TAI */ + "tcb", /* ASDF_TIME_SCALE_TCB */ + "tcg", /* ASDF_TIME_SCALE_TCG */ + "tdb", /* ASDF_TIME_SCALE_TDB */ + "tt", /* ASDF_TIME_SCALE_TT */ + "ut1", /* ASDF_TIME_SCALE_UT1 */ +}; + + +static asdf_value_t *asdf_time_serialize( + asdf_file_t *file, const void *obj, UNUSED(const void *userdata)) { + + if (UNLIKELY(!file || !obj)) + return NULL; + + const asdf_time_t *t = obj; + asdf_mapping_t *map = NULL; + asdf_value_t *value = NULL; + asdf_value_err_t err = ASDF_VALUE_ERR_EMIT_FAILURE; + + if (!t->value) { + ASDF_LOG(file, ASDF_LOG_WARN, ASDF_CORE_TIME_TAG " requires a value"); + goto cleanup; + } + + const size_t nformats = sizeof(asdf_time_format_names) / sizeof(asdf_time_format_names[0]); + if ((size_t)t->format.type >= nformats || !asdf_time_format_names[t->format.type]) { + ASDF_LOG(file, ASDF_LOG_WARN, ASDF_CORE_TIME_TAG " unknown or reserved format type"); + goto cleanup; + } + + map = asdf_mapping_create(file); + if (!map) + goto cleanup; + + err = asdf_mapping_set_string0(map, "value", t->value); + if (err != ASDF_VALUE_OK) + goto cleanup; + + err = asdf_mapping_set_string0(map, "format", asdf_time_format_names[t->format.type]); + if (err != ASDF_VALUE_OK) + goto cleanup; + + /* Write scale only if non-UTC */ + if (t->scale != ASDF_TIME_SCALE_UTC) { + const size_t nscales = sizeof(asdf_time_scale_names) / sizeof(asdf_time_scale_names[0]); + if ((size_t)t->scale < nscales) { + err = asdf_mapping_set_string0(map, "scale", asdf_time_scale_names[t->scale]); + if (err != ASDF_VALUE_OK) + goto cleanup; + } + } + + /* Write location only if any field is non-zero */ + if (t->location.longitude != 0.0 || t->location.latitude != 0.0 || t->location.height != 0.0) { + asdf_mapping_t *loc_map = asdf_mapping_create(file); + if (!loc_map) + goto cleanup; + + err = asdf_mapping_set_double(loc_map, "longitude", t->location.longitude); + if (err != ASDF_VALUE_OK) { + asdf_mapping_destroy(loc_map); + goto cleanup; + } + err = asdf_mapping_set_double(loc_map, "latitude", t->location.latitude); + if (err != ASDF_VALUE_OK) { + asdf_mapping_destroy(loc_map); + goto cleanup; + } + err = asdf_mapping_set_double(loc_map, "height", t->location.height); + if (err != ASDF_VALUE_OK) { + asdf_mapping_destroy(loc_map); + goto cleanup; + } + + err = asdf_mapping_set_mapping(map, "location", loc_map); + if (err != ASDF_VALUE_OK) { + asdf_mapping_destroy(loc_map); + goto cleanup; + } + } + + value = asdf_value_of_mapping(map); + +cleanup: + if (err != ASDF_VALUE_OK) + asdf_mapping_destroy(map); + + return value; +} + + +static asdf_value_err_t asdf_time_deserialize( + asdf_value_t *value, UNUSED(const void *userdata), void **out) { + const char *value_s = NULL; + const char *format_s = NULL; + + asdf_mapping_t *mapping = NULL; + asdf_value_t *prop = NULL; + asdf_value_err_t err = ASDF_VALUE_ERR_PARSE_FAILURE; + + asdf_time_t *time = calloc(1, sizeof(asdf_time_t)); + if (!time) { + return ASDF_VALUE_ERR_OOM; + } + + if (asdf_value_is_mapping(value)) { + if (asdf_value_as_mapping(value, &mapping) != ASDF_VALUE_OK) + goto failure; + prop = asdf_mapping_get(mapping, "value"); + } else { + prop = value; + } + + time->value = calloc(ASDF_TIME_TIMESTR_MAXLEN, sizeof(*time->value)); + if (!time->value) { + free(time); + return ASDF_VALUE_ERR_OOM; + } + + const asdf_value_type_t type = asdf_value_get_type(prop); + switch (type) { + case ASDF_VALUE_INT64: { + time_t value_tmp = 0; + asdf_value_as_int64(value, &value_tmp); + snprintf(time->value, ASDF_TIME_TIMESTR_MAXLEN, "%ld", value_tmp); + } break; + case ASDF_VALUE_DOUBLE: { + double value_tmp = 0.0; + asdf_value_as_double(value, &value_tmp); + snprintf(time->value, ASDF_TIME_TIMESTR_MAXLEN, "%lf", value_tmp); + } break; + case ASDF_VALUE_FLOAT: { + float value_tmp = 0.0f; + asdf_value_as_float(value, &value_tmp); + snprintf(time->value, ASDF_TIME_TIMESTR_MAXLEN, "%f", value_tmp); + } break; + case ASDF_VALUE_STRING: { + asdf_value_as_string0(prop, &value_s); + strncpy(time->value, value_s, ASDF_TIME_TIMESTR_MAXLEN - 1); + } break; + default: + fprintf(stderr, "unhandled property conversion from scalar enum %d\n", prop->type); + goto failure; + } + + if (prop != value) { + asdf_value_destroy(prop); + prop = NULL; + } + + if (mapping) { + prop = asdf_mapping_get(mapping, "format"); + if (ASDF_VALUE_OK != asdf_value_as_string0(prop, &format_s)) { + goto failure; + } + asdf_value_destroy(prop); + prop = NULL; + } + + const char *time_auto_keys[] = { + "iso_time", + "byear", + "jyear", + "yday", + }; + + const char *time_auto_patterns[] = { + /* iso_time */ + "[0-9]{4}-(0[1-9])|(1[0-2])-(0[1-9])|([1-2][0-9])|(3[0-1])" + "[T ]([0-1][0-9])|(2[0-4]):[0-5][0-9]:[0-5][0-9](.[0-9]+)?", + /* byear */ + "B[0-9]+(.[0-9]+)?", + /* jyear */ + "J[0-9]+(.[0-9]+)?", + /* yday */ + "[0-9]{4}:(00[1-9])|(0[1-9][0-9])|([1-2][0-9][0-9])|(3[0-5][0-9])|(36[0-5]):([0-1][0-9])|" + "([0-1][0-9])|(2[0-4]):[0-5][0-9]:[0-5][0-9](.[0-9]+)?"}; + + time->format.is_base_format = true; + for (size_t idx = 0; idx < sizeof(time_auto_patterns) / sizeof(time_auto_patterns[0]); idx++) { + cregex re = cregex_make(time_auto_patterns[idx], CREG_DEFAULT); + if (cregex_is_match(&re, time->value) == true) { + const char *fmt_have = format_s ? format_s : time_auto_keys[idx]; + if (!strcmp(fmt_have, "iso_time")) { + time->format.type = ASDF_TIME_FORMAT_ISO_TIME; + } else if (!strcmp(fmt_have, "byear") || !strncmp(time->value, "B", 1)) { + time->format.type = ASDF_TIME_FORMAT_BYEAR; + } else if (!strcmp(fmt_have, "jd")) { + time->format.type = ASDF_TIME_FORMAT_JD; + } else if (!strcmp(fmt_have, "mjd")) { + time->format.type = ASDF_TIME_FORMAT_MJD; + } else if (!strcmp(fmt_have, "jyear") || !strncmp(time->value, "J", 1)) { + time->format.type = ASDF_TIME_FORMAT_JYEAR; + } else if (!strcmp(fmt_have, "yday")) { + time->format.type = ASDF_TIME_FORMAT_YDAY; + } else if (!strcmp(fmt_have, "unix")) { + time->format.type = ASDF_TIME_FORMAT_UNIX; + } + time->scale = ASDF_TIME_SCALE_UTC; + cregex_drop(&re); + break; + } + cregex_drop(&re); + } + + if (time->format.type > ASDF_TIME_FORMAT_RESERVED1) { + time->format.is_base_format = false; + } + + asdf_time_parse_time(time->value, &time->format, &time->info); + + *out = time; + + return ASDF_VALUE_OK; +failure: + free(time->value); + free(time); + asdf_value_destroy(prop); + return err; +} + + +static void *asdf_time_copy(const void *obj) { + if (!obj) + return NULL; + + const asdf_time_t *t = obj; + asdf_time_t *copy = calloc(1, sizeof(asdf_time_t)); + + if (!copy) + return NULL; + + *copy = *t; + copy->value = t->value ? strdup(t->value) : NULL; + + return copy; +} + + +static void asdf_time_dealloc(void *value) { + asdf_time_t *t = (asdf_time_t *)value; + free(t->value); + free(t); +} + + +ASDF_REGISTER_EXTENSION( + time, + ASDF_CORE_TIME_TAG, + asdf_time_t, + &libasdf_software, + asdf_time_serialize, + asdf_time_deserialize, + asdf_time_copy, + asdf_time_dealloc, + NULL); From 9c8d138ac7c3014e94d602e39aa0dc2889f8d280 Mon Sep 17 00:00:00 2001 From: "E. Madison Bray" <676149+embray@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:17:02 +0100 Subject: [PATCH 2/5] test: use per-process-group run directory for temp files Replace mkstemp-based temp file paths (random names scattered in TEMP_DIR) with per-run numbered subdirectories (TEMP_DIR/000001/, 000002/, ...). All test binaries that share a process group (i.e. all binaries launched by a single `make check` invocation) land in the same subdirectory via a PGID coordination file (TEMP_DIR/.pgid-). The first binary to start is "pioneer" (wins an O_CREAT|O_EXCL race on the coordination file) and creates the run directory; subsequent binaries are "followers" that read the serial from the coordination file and join the same directory. Coordination files are cleaned up lazily: on startup each binary removes files whose process group is no longer alive (kill(-pgid, 0) == ESRCH). Empty run directories are reclaimed on exit via a destructor that calls rmdir (no-op if non-empty). The pioneer also reuses the most recent run directory if it is empty, so clean runs do not accumulate directories. A "latest" symlink points to the most recently started run for convenience. test: fix get_total_memory call in parse util tests --- tests/Makefile.am | 4 +- tests/munit.h | 14 ++- tests/test-tests.c | 17 +-- tests/util.c | 252 +++++++++++++++++++++++++++++++++++++++++---- tests/util.h | 1 + 5 files changed, 256 insertions(+), 32 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 32addb55..e384cb60 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -142,7 +142,7 @@ test_parse_util_unit_SOURCES = \ test_parse_util_unit_CPPFLAGS = $(unit_test_cppflags) test_parse_util_unit_CFLAGS = $(unit_test_cflags) test_parse_util_unit_LDFLAGS = $(unit_test_ldflags) -test_parse_util_unit_LDADD = libmunit.a $(FYAML_LIBS) $(MD5_LIBS) +test_parse_util_unit_LDADD = libmunit.a $(FYAML_LIBS) $(STATGRAB_LIBS) $(MD5_LIBS) if HAVE_LINKER_WRAP # test-malloc-fail.unit @@ -212,7 +212,7 @@ test_value_util_unit_SOURCES = \ test_value_util_unit_CPPFLAGS = $(unit_test_cppflags) test_value_util_unit_CFLAGS = $(unit_test_cflags) test_value_util_unit_LDFLAGS = $(unit_test_ldflags) -test_value_util_unit_LDADD = libmunit.a +test_value_util_unit_LDADD = libmunit.a $(STATGRAB_LIBS) # test-yaml.unit test_yaml_unit_SOURCES = \ diff --git a/tests/munit.h b/tests/munit.h index 153ee032..7ed2837d 100644 --- a/tests/munit.h +++ b/tests/munit.h @@ -120,18 +120,26 @@ static inline void mu_cleanup_tempfiles(const fixtures *fix) { if (getenv("ASDF_TEST_KEEP_TEMP")) return; - DIR *d = opendir(TEMP_DIR); + const char *run_dir = get_run_dir(); + DIR *d = opendir(run_dir); if (!d) return; const char *prefix = fix->tempfile_prefix; size_t prefix_len = strlen(prefix); + /* Strip trailing '-': get_temp_file_path drops it before '.' suffixes, + * so filenames are "suite-test.asdf" not "suite-test-.asdf". + * Match on the base name, then confirm the next char is '-' or '.' */ + size_t match_len = (prefix_len > 0 && prefix[prefix_len - 1] == '-') + ? prefix_len - 1 + : prefix_len; struct dirent *ent; while ((ent = readdir(d)) != NULL) { - if (strncmp(ent->d_name, prefix, prefix_len) == 0) { + char c = ent->d_name[match_len]; + if (strncmp(ent->d_name, prefix, match_len) == 0 && (c == '-' || c == '.')) { char fullpath[PATH_MAX]; - snprintf(fullpath, sizeof(fullpath), "%s/%s", TEMP_DIR, ent->d_name); + snprintf(fullpath, sizeof(fullpath), "%s/%s", run_dir, ent->d_name); unlink(fullpath); /* ignore errors */ } } diff --git a/tests/test-tests.c b/tests/test-tests.c index 53a5b213..dcd0c384 100644 --- a/tests/test-tests.c +++ b/tests/test-tests.c @@ -31,14 +31,19 @@ MU_TEST(test_test_name_2) { MU_TEST(test_get_temp_file_path) { const char *filename = get_temp_file_path(fixture->tempfile_prefix, ".asdf"); assert_not_null(filename); + /* File should exist and live under the run directory */ struct stat st; - assert_int(stat(TEMP_DIR, &st), !=, -1); - size_t temp_dir_len = strlen(TEMP_DIR); + assert_int(stat(filename, &st), ==, 0); + const char *run_dir = get_run_dir(); + size_t run_dir_len = strlen(run_dir); size_t prefix_len = strlen(fixture->tempfile_prefix); - assert_int(strlen(filename), ==, temp_dir_len + 1 + prefix_len + 6 + 5); - assert_memory_equal(temp_dir_len, filename, TEMP_DIR); - assert_memory_equal(prefix_len, filename + 1 + temp_dir_len, fixture->tempfile_prefix); - assert_memory_equal(5, filename + 1 + temp_dir_len + prefix_len + 6, ".asdf"); + /* get_temp_file_path strips the trailing '-' before a '.' suffix, so the + * filename is run_dir + "/" + prefix_without_trailing_dash + ".asdf" */ + assert_int(strlen(filename), ==, run_dir_len + 1 + (prefix_len - 1) + 5); + assert_memory_equal(run_dir_len, filename, run_dir); + assert_char(filename[run_dir_len], ==, '/'); + assert_memory_equal(prefix_len - 1, filename + run_dir_len + 1, fixture->tempfile_prefix); + assert_string_equal(filename + run_dir_len + 1 + prefix_len - 1, ".asdf"); return MUNIT_OK; } diff --git a/tests/util.c b/tests/util.c index c992cf09..efcd3e03 100644 --- a/tests/util.c +++ b/tests/util.c @@ -2,7 +2,11 @@ * Utilities for unit tests */ +#include +#include +#include #include +#include #include #include #include @@ -31,7 +35,7 @@ size_t get_total_memory(void) { #ifndef HAVE_STATGRAB return 0; #else - sg_init(1); // TODO: Maybe move this to somewhere else like during library init + sg_init(1); size_t entries = 0; sg_mem_stats *mem_stats = sg_get_mem_stats(&entries); sg_shutdown(); @@ -66,38 +70,244 @@ static void ensure_tmp_dir(void) { } -const char *get_temp_file_path(const char *prefix, const char *suffix) { - ensure_tmp_dir(); +/** + * Per-run directory: TEMP_DIR/{:06d}/ + * + * All test binaries that share a process group (i.e. all binaries launched by + * a single `make check` invocation) land in the same numbered subdirectory. + * Sequential numbering makes it easy to compare between recent runs. The + * "latest" symlink always points to the most recently started run. + * + * A PGID coordination file (TEMP_DIR/.pgid-) records the run serial and + * persists for as long as the process group is alive. When a test binary + * starts it checks whether a coordination file exists for its PGID: + * + * - No file (pioneer): create the next numbered run directory, write the + * serial to the coordination file, and update the "latest" symlink. + * - File present (follower): read the serial and reuse the same directory. + * + * At startup each binary also lazily removes coordination files whose process + * groups are no longer alive (kill(-pgid, 0) == ESRCH). + * + * Note: in non-interactive shells without job control, `make` inherits the + * invoking shell's PGID rather than creating its own, so two sequential + * `make check` calls in the same shell session may share a PGID. The stale + * check only fires once the shell exits, so the second run would reuse the + * first run's directory. This edge case is uncommon enough in practice to + * leave unaddressed for now. + */ +static char run_dir_storage[PATH_MAX]; - static char path[PATH_MAX]; - static char fullpath[PATH_MAX]; - int n = snprintf(path, sizeof(path), TEMP_DIR "/%sXXXXXX", prefix ? prefix : ""); +#define PGID_FILE_TEMPLATE ".pgid-%d" - if (n < 0 || n >= sizeof(path)) - return NULL; - int fd = mkstemp(path); +/* Remove coordination files whose process groups no longer exist. */ +static void clean_stale_pgid_files(void) { + DIR *d = opendir(TEMP_DIR); + if (!d) + return; - if (fd < 0) - return NULL; + struct dirent *ent; + while ((ent = readdir(d)) != NULL) { + int pgid = 0; + if (sscanf(ent->d_name, PGID_FILE_TEMPLATE, &pgid) != 1 || pgid <= 0) + continue; - close(fd); + if (kill(-(pid_t)pgid, 0) == -1 && errno == ESRCH) { + char path[PATH_MAX]; + snprintf(path, sizeof(path), TEMP_DIR "/%s", ent->d_name); + unlink(path); + } + } - if (suffix) { - n = snprintf(fullpath, sizeof(fullpath), "%s%s", path, suffix); + closedir(d); +} - if (n < 0 || n >= sizeof(fullpath)) - return NULL; - if (rename(path, fullpath) != 0) { - unlink(path); - return NULL; +__attribute__((constructor)) +static void init_run_dir(void) { + ensure_tmp_dir(); + clean_stale_pgid_files(); + + pid_t pgid = getpgrp(); + char pgid_file[PATH_MAX]; + snprintf(pgid_file, sizeof(pgid_file), TEMP_DIR "/" PGID_FILE_TEMPLATE, (int)pgid); + + /* + * Follower path: if a coordination file already exists for this PGID, + * read the run serial from it and reuse the same directory. + */ + { + int fd = open(pgid_file, O_RDONLY); + if (fd >= 0) { + char serial_str[7] = {0}; + ssize_t n = read(fd, serial_str, sizeof(serial_str) - 1); + close(fd); + if (n > 0) { + snprintf(run_dir_storage, sizeof(run_dir_storage), + TEMP_DIR "/%s", serial_str); + return; + } + /* File exists but is empty: pioneer hasn't written yet; fall through + * to a brief retry below rather than creating a competing directory. */ + } + } + + /* + * Pioneer path: atomically claim the coordination file. If O_EXCL fails + * with EEXIST the pioneer beat us; retry reading with a short backoff. + */ + int fd_create = open(pgid_file, O_WRONLY | O_CREAT | O_EXCL, 0600); + + if (fd_create < 0) { + if (errno != EEXIST) + return; /* unexpected error; get_run_dir() will fall back to TEMP_DIR */ + + /* Lost the race to the pioneer; wait for it to write the serial. */ + for (int attempt = 0; attempt < 200; attempt++) { + usleep(5000); + int fd = open(pgid_file, O_RDONLY); + if (fd < 0) + continue; + char serial_str[7] = {0}; + ssize_t n = read(fd, serial_str, sizeof(serial_str) - 1); + close(fd); + if (n > 0) { + snprintf(run_dir_storage, sizeof(run_dir_storage), + TEMP_DIR "/%s", serial_str); + return; + } + } + return; /* timed out; get_run_dir() falls back to TEMP_DIR */ + } + + /* + * We are the pioneer. Before creating a new numbered directory, check + * whether the "latest" run directory is empty. If it is, reuse that + * serial so that clean runs do not accumulate empty directories. + */ + { + char latest_link[PATH_MAX]; + char serial_str[7] = {0}; + snprintf(latest_link, sizeof(latest_link), TEMP_DIR "/latest"); + if (readlink(latest_link, serial_str, sizeof(serial_str) - 1) > 0) { + char candidate[PATH_MAX]; + snprintf(candidate, sizeof(candidate), TEMP_DIR "/%s", serial_str); + DIR *ld = opendir(candidate); + if (ld) { + int is_empty = 1; + struct dirent *lent; + while ((lent = readdir(ld)) != NULL) { + if (strcmp(lent->d_name, ".") != 0 + && strcmp(lent->d_name, "..") != 0) { + is_empty = 0; + break; + } + } + closedir(ld); + if (is_empty) { + /* Reuse the existing empty directory. */ + snprintf(run_dir_storage, sizeof(run_dir_storage), + "%s", candidate); + (void)write(fd_create, serial_str, strlen(serial_str)); + close(fd_create); + /* "latest" already points here; no need to update. */ + return; + } + } + } + } + + /* Scan for the highest existing numbered run directory. */ + int run_num = 0; + DIR *d = opendir(TEMP_DIR); + if (d) { + struct dirent *ent; + while ((ent = readdir(d)) != NULL) { + int num = 0; + if (strlen(ent->d_name) == 6 + && sscanf(ent->d_name, "%6d", &num) == 1 + && num > run_num) + run_num = num; } + closedir(d); + } - return fullpath; + /* Create the next sequential run directory (mkdir is atomic on POSIX). */ + for (run_num++; run_num < 1000000; run_num++) { + if (snprintf(run_dir_storage, sizeof(run_dir_storage), + TEMP_DIR "/%06d", run_num) >= (int)sizeof(run_dir_storage)) { + run_dir_storage[0] = '\0'; + close(fd_create); + unlink(pgid_file); + return; + } + if (mkdir(run_dir_storage, 0777) == 0) + break; + if (errno != EEXIST) { + run_dir_storage[0] = '\0'; + close(fd_create); + unlink(pgid_file); + return; + } } - return path; + /* Write the serial into the coordination file and update "latest". */ + char serial_str[7]; + snprintf(serial_str, sizeof(serial_str), "%06d", run_num); + (void)write(fd_create, serial_str, strlen(serial_str)); + close(fd_create); + + char latest[PATH_MAX]; + snprintf(latest, sizeof(latest), TEMP_DIR "/latest"); + unlink(latest); + symlink(serial_str, latest); /* best-effort */ +} + + +/* On exit, attempt to remove the run directory if it is empty. This is + * best-effort: rmdir silently fails on a non-empty directory, and with + * parallel test execution (make -jN) multiple binaries share the directory, + * so only the last binary to exit will succeed if no files were left. */ +__attribute__((destructor)) +static void cleanup_run_dir(void) { + if (run_dir_storage[0]) + rmdir(run_dir_storage); /* no-op if non-empty */ +} + + +const char *get_run_dir(void) { + /* Fallback to TEMP_DIR if the run directory could not be created. */ + return run_dir_storage[0] ? run_dir_storage : TEMP_DIR; +} + + +const char *get_temp_file_path(const char *prefix, const char *suffix) { + static char fullpath[PATH_MAX]; + + /* When prefix ends with '-' and suffix starts with '.' (e.g. ".asdf"), + * strip the trailing '-' so we get "test-name.asdf" not "test-name-.asdf" */ + size_t plen = prefix ? strlen(prefix) : 0; + if (plen > 0 && prefix[plen - 1] == '-' && suffix && suffix[0] == '.') + plen--; + + int n = snprintf(fullpath, sizeof(fullpath), "%s/%.*s%s", + get_run_dir(), (int)plen, prefix ? prefix : "", + suffix ? suffix : ""); + + if (n < 0 || n >= (int)sizeof(fullpath)) + return NULL; + + /* Ensure the run directory exists: the destructor may have removed it if + * it was empty between the last test binary's exit and this call. */ + mkdir(get_run_dir(), 0777); /* no-op if it already exists */ + + /* Create the file so it exists (matching the old mkstemp-based behaviour). */ + int fd = open(fullpath, O_CREAT | O_WRONLY, 0600); + if (fd >= 0) + close(fd); + + return fullpath; } diff --git a/tests/util.h b/tests/util.h index 1071e483..2ac83f6d 100644 --- a/tests/util.h +++ b/tests/util.h @@ -17,6 +17,7 @@ size_t get_total_memory(void); const char *get_fixture_file_path(const char *relative_path); const char *get_reference_file_path(const char *relative_path); +const char *get_run_dir(void); const char *get_temp_file_path(const char *prefix, const char *suffix); char *read_file(const char *filename, size_t *out_len); char *tail_file(const char *filename, uint32_t skip, size_t *out_len); From a7e03d3aba255c1c22964e6d7168f28cfe34f20a Mon Sep 17 00:00:00 2001 From: "E. Madison Bray" <676149+embray@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:29:03 +0100 Subject: [PATCH 3/5] test: add tests for time extension and fix core extensions test --- tests/Makefile.am | 8 +++ tests/fixtures/time.asdf | 22 +++++++ tests/test-core-extensions.c | 5 +- tests/test-time.c | 121 +++++++++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/time.asdf create mode 100644 tests/test-time.c diff --git a/tests/Makefile.am b/tests/Makefile.am index e384cb60..d9d9a0cc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -37,6 +37,7 @@ check_PROGRAMS = \ test-parser.unit \ test-stream.unit \ test-tests.unit \ + test-time.unit \ test-value.unit \ test-value-util.unit \ test-yaml.unit @@ -197,6 +198,13 @@ test_tests_unit_CFLAGS = $(unit_test_cflags) test_tests_unit_LDFLAGS = $(unit_test_ldflags) test_tests_unit_LDADD = libmunit.a $(STATGRAB_LIBS) +# test-time.unit +test_time_unit_SOURCES = test-time.c +test_time_unit_CPPFLAGS = $(unit_test_cppflags) +test_time_unit_CFLAGS = $(unit_test_cflags) +test_time_unit_LDFLAGS = $(unit_test_ldflags) +test_time_unit_LDADD = $(unit_test_ldadd) + # test-value.unit test_value_unit_SOURCES = test-value.c test_value_unit_CPPFLAGS = $(unit_test_cppflags) diff --git a/tests/fixtures/time.asdf b/tests/fixtures/time.asdf new file mode 100644 index 00000000..9c2f225f --- /dev/null +++ b/tests/fixtures/time.asdf @@ -0,0 +1,22 @@ +#ASDF 1.0.0 +#ASDF_STANDARD 1.6.0 +%YAML 1.1 +%TAG ! tag:stsci.edu:asdf/ +--- !core/asdf-1.1.0 +asdf_library: !core/software-1.0.0 {author: The ASDF Developers, homepage: 'http://github.com/asdf-format/asdf', + name: asdf, version: 5.0.0} +history: + extensions: + - !core/extension_metadata-1.0.0 + extension_class: asdf.extension._manifest.ManifestExtension + extension_uri: asdf://asdf-format.org/core/extensions/core-1.6.0 + manifest_software: !core/software-1.0.0 {name: asdf_standard, version: 1.4.0} + software: !core/software-1.0.0 {name: asdf, version: 5.0.0} +t_byear: !time/time-1.4.0 {format: byear, value: '2025.78707178'} +t_datetime: !time/time-1.4.0 {format: datetime, value: '2025-10-14 13:26:41.0000+00:00'} +t_iso_time: !time/time-1.4.0 {format: iso_time, value: '2025-10-14T13:26:41.0000'} +t_jd: !time/time-1.4.0 {format: jd, value: '2460963.060197'} +t_mjd: !time/time-1.4.0 {format: mjd, value: '60962.560196'} +t_unix: !time/time-1.4.0 {format: unix, value: '1760462801.00'} +t_yday: !time/time-1.4.0 {format: yday, value: '2025:287:13:26:41.0000'} +... diff --git a/tests/test-core-extensions.c b/tests/test-core-extensions.c index 375b5fdf..d54efc55 100644 --- a/tests/test-core-extensions.c +++ b/tests/test-core-extensions.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -184,8 +185,8 @@ MU_TEST(history_entry) { assert_not_null(entry); assert_string_equal(entry->description, "test file containing integers from 0 to 255 in the " "block data, for simple tests against known data"); - assert_int(entry->time.tv_sec, ==, 1753271775); - assert_int(entry->time.tv_nsec, ==, 0); + assert_int(entry->time->info.ts.tv_sec, ==, 1753271775); + assert_int(entry->time->info.ts.tv_nsec, ==, 0); // TODO: Oops, current test case does not include software used to write the history entry // A little strange that Python asdf excludes it...maybe no one cares because it's the only // code writing asdf history entries? Need more test cases... diff --git a/tests/test-time.c b/tests/test-time.c new file mode 100644 index 00000000..bfe18c47 --- /dev/null +++ b/tests/test-time.c @@ -0,0 +1,121 @@ +#include +#include + +#include "munit.h" +#include "util.h" + +#include +#include +#include +#include + + +MU_TEST(test_asdf_time) { + const char *path = get_fixture_file_path("time.asdf"); + assert_not_null(path); + + asdf_file_t *file = asdf_open(path, "r"); + assert_not_null(file); + + asdf_value_t *value = NULL; + + /* buffer for formatted time string */ + char time_str[255] = {0}; + + const char *fixture_keys[] = { + "t_iso_time", + "t_datetime", + "t_yday", + "t_unix", + "t_jd", + "t_mjd", + "t_byear", + }; + + asdf_time_t *t = NULL; + for (size_t idx = 0; idx < sizeof(fixture_keys) / sizeof(fixture_keys[0]); idx++) { + const char *key = fixture_keys[idx]; + assert_true(asdf_is_time(file, key)); + + value = asdf_get_value(file, key); + assert_not_null(value); + + asdf_value_err_t err = asdf_value_as_time(value, &t); + if (err != ASDF_VALUE_OK) { + munit_logf(MUNIT_LOG_ERROR, "asdf_value_as_time failed: %s\n", key); + asdf_time_destroy(t); + asdf_value_destroy(value); + asdf_close(file); + return MUNIT_FAIL; + } + + assert_not_null(t); + assert_not_null(t->value); + + time_t x = t->info.ts.tv_sec; + strftime(time_str, sizeof(time_str), "%m/%d/%Y %T %Z", gmtime(&x)); + printf("[%zu] key: %10s, value: %30s, time: %10s\n", idx, key, t->value, time_str); + + asdf_time_destroy(t); + t = NULL; + memset(time_str, 0, sizeof(time_str)); + asdf_value_destroy(value); + value = NULL; + } + + asdf_close(file); + + return MUNIT_OK; +} + + +MU_TEST(test_asdf_time_serialize) { + const char *path = get_temp_file_path(fixture->tempfile_prefix, ".asdf"); + assert_not_null(path); + + /* Write a time value to a new file */ + asdf_file_t *file = asdf_open(NULL); + assert_not_null(file); + + char time_value[] = "2025-10-14T13:26:41.0000"; + asdf_time_t time_obj = { + .value = time_value, + .format = {.is_base_format = true, .type = ASDF_TIME_FORMAT_ISO_TIME}, + .scale = ASDF_TIME_SCALE_UTC, + }; + + asdf_value_err_t err = asdf_set_time(file, "t_write", &time_obj); + assert_int(err, ==, ASDF_VALUE_OK); + + assert_int(asdf_write_to(file, path), ==, 0); + asdf_close(file); + + /* Re-open and verify the round-trip */ + file = asdf_open(path, "r"); + assert_not_null(file); + + assert_true(asdf_is_time(file, "t_write")); + + asdf_time_t *t_out = NULL; + err = asdf_get_time(file, "t_write", &t_out); + assert_int(err, ==, ASDF_VALUE_OK); + assert_not_null(t_out); + assert_not_null(t_out->value); + assert_string_equal(t_out->value, time_value); + assert_int(t_out->format.type, ==, ASDF_TIME_FORMAT_ISO_TIME); + + asdf_time_destroy(t_out); + asdf_close(file); + + return MUNIT_OK; +} + + +MU_TEST_SUITE( + test_asdf_time_extension, + MU_RUN_TEST(test_asdf_time), + MU_RUN_TEST(test_asdf_time_serialize) +); + + +MU_RUN_SUITE(test_asdf_time_extension); From d3086c49291c08636b127e84a0a5a743bba20c44 Mon Sep 17 00:00:00 2001 From: "E. Madison Bray" <676149+embray@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:48:46 +0100 Subject: [PATCH 4/5] style: clean up `clang tidy` warnings in tests/util.c; splitting up init_run_dir into cleaner subroutines --- .clang-tidy | 2 +- tests/util.c | 273 ++++++++++++++++++++++++++++----------------------- 2 files changed, 152 insertions(+), 123 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 37ce65ed..3a578520 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -20,7 +20,7 @@ CheckOptions: - key: readability-identifier-length.IgnoredParameterNames value: '^fd|fp$' - key: readability-identifier-length.IgnoredVariableNames - value: '^fd|p|q|st|tm|tz$' + value: '^fd|n|p|q|st|tm|tz$' - key: readability-identifier-naming.EnumCase value: lower_case - key: readability-identifier-naming.FunctionCase diff --git a/tests/util.c b/tests/util.c index efcd3e03..33ff6bec 100644 --- a/tests/util.c +++ b/tests/util.c @@ -103,12 +103,12 @@ static char run_dir_storage[PATH_MAX]; /* Remove coordination files whose process groups no longer exist. */ static void clean_stale_pgid_files(void) { - DIR *d = opendir(TEMP_DIR); - if (!d) + DIR *dir = opendir(TEMP_DIR); + if (!dir) return; struct dirent *ent; - while ((ent = readdir(d)) != NULL) { + while ((ent = readdir(dir)) != NULL) { int pgid = 0; if (sscanf(ent->d_name, PGID_FILE_TEMPLATE, &pgid) != 1 || pgid <= 0) continue; @@ -120,148 +120,177 @@ static void clean_stale_pgid_files(void) { } } - closedir(d); + closedir(dir); } -__attribute__((constructor)) -static void init_run_dir(void) { - ensure_tmp_dir(); - clean_stale_pgid_files(); +#define TEST_SERIAL_LEN 6 +#define TEST_SERIAL_FMT "%6d" +#define TEST_SERIAL_MAX 1000000 - pid_t pgid = getpgrp(); - char pgid_file[PATH_MAX]; - snprintf(pgid_file, sizeof(pgid_file), TEMP_DIR "/" PGID_FILE_TEMPLATE, (int)pgid); - /* - * Follower path: if a coordination file already exists for this PGID, - * read the run serial from it and reuse the same directory. - */ - { - int fd = open(pgid_file, O_RDONLY); - if (fd >= 0) { - char serial_str[7] = {0}; - ssize_t n = read(fd, serial_str, sizeof(serial_str) - 1); - close(fd); - if (n > 0) { - snprintf(run_dir_storage, sizeof(run_dir_storage), - TEMP_DIR "/%s", serial_str); - return; - } - /* File exists but is empty: pioneer hasn't written yet; fall through - * to a brief retry below rather than creating a competing directory. */ - } +/* + * Try to read the run serial from an existing PGID coordination file. + * Returns 1 and sets run_dir_storage on success, 0 otherwise. + */ +static int join_existing_run(const char *pgid_file) { + int fd = open(pgid_file, O_RDONLY); + if (fd < 0) + return 0; + + char serial_str[TEST_SERIAL_LEN + 1] = {0}; + ssize_t n = read(fd, serial_str, sizeof(serial_str) - 1); + close(fd); + + if (n <= 0) + return 0; + + snprintf(run_dir_storage, sizeof(run_dir_storage), TEMP_DIR "/%s", serial_str); + return 1; +} + + +#define WAIT_FOR_PIONEER_ATTEMPTS 200 +#define WAIT_FOR_PIONEER_DELAY 5000 // usec + + +/* + * Retry joining a run after losing the O_EXCL race to the pioneer. + * Polls the coordination file with a short backoff until the pioneer + * writes the serial. + */ +static void wait_for_pioneer(const char *pgid_file) { + for (int attempt = 0; attempt < WAIT_FOR_PIONEER_ATTEMPTS; attempt++) { + usleep(WAIT_FOR_PIONEER_DELAY); + if (join_existing_run(pgid_file)) + return; } + /* Timed out; get_run_dir() falls back to TEMP_DIR. */ +} - /* - * Pioneer path: atomically claim the coordination file. If O_EXCL fails - * with EEXIST the pioneer beat us; retry reading with a short backoff. - */ - int fd_create = open(pgid_file, O_WRONLY | O_CREAT | O_EXCL, 0600); - if (fd_create < 0) { - if (errno != EEXIST) - return; /* unexpected error; get_run_dir() will fall back to TEMP_DIR */ - - /* Lost the race to the pioneer; wait for it to write the serial. */ - for (int attempt = 0; attempt < 200; attempt++) { - usleep(5000); - int fd = open(pgid_file, O_RDONLY); - if (fd < 0) - continue; - char serial_str[7] = {0}; - ssize_t n = read(fd, serial_str, sizeof(serial_str) - 1); - close(fd); - if (n > 0) { - snprintf(run_dir_storage, sizeof(run_dir_storage), - TEMP_DIR "/%s", serial_str); - return; - } +/* + * If the "latest" symlink points to an existing empty directory, set + * run_dir_storage to that directory and copy its serial into serial_out. + * Returns 1 on success (caller should reuse the directory), 0 otherwise. + */ +static int try_reuse_latest(char serial_out[TEST_SERIAL_LEN + 1]) { + char latest_link[PATH_MAX]; + snprintf(latest_link, sizeof(latest_link), TEMP_DIR "/latest"); + + char serial_str[TEST_SERIAL_LEN + 1] = {0}; + if (readlink(latest_link, serial_str, sizeof(serial_str) - 1) <= 0) + return 0; + + char candidate[PATH_MAX]; + snprintf(candidate, sizeof(candidate), TEMP_DIR "/%s", serial_str); + + DIR *dir = opendir(candidate); + if (!dir) + return 0; + + int is_empty = 1; + struct dirent *ent; + while ((ent = readdir(dir)) != NULL) { + if (strcmp(ent->d_name, ".") != 0 && strcmp(ent->d_name, "..") != 0) { + is_empty = 0; + break; } - return; /* timed out; get_run_dir() falls back to TEMP_DIR */ } + closedir(dir); - /* - * We are the pioneer. Before creating a new numbered directory, check - * whether the "latest" run directory is empty. If it is, reuse that - * serial so that clean runs do not accumulate empty directories. - */ - { - char latest_link[PATH_MAX]; - char serial_str[7] = {0}; - snprintf(latest_link, sizeof(latest_link), TEMP_DIR "/latest"); - if (readlink(latest_link, serial_str, sizeof(serial_str) - 1) > 0) { - char candidate[PATH_MAX]; - snprintf(candidate, sizeof(candidate), TEMP_DIR "/%s", serial_str); - DIR *ld = opendir(candidate); - if (ld) { - int is_empty = 1; - struct dirent *lent; - while ((lent = readdir(ld)) != NULL) { - if (strcmp(lent->d_name, ".") != 0 - && strcmp(lent->d_name, "..") != 0) { - is_empty = 0; - break; - } - } - closedir(ld); - if (is_empty) { - /* Reuse the existing empty directory. */ - snprintf(run_dir_storage, sizeof(run_dir_storage), - "%s", candidate); - (void)write(fd_create, serial_str, strlen(serial_str)); - close(fd_create); - /* "latest" already points here; no need to update. */ - return; - } - } - } + if (!is_empty) + return 0; + + snprintf(run_dir_storage, sizeof(run_dir_storage), "%s", candidate); + snprintf(serial_out, TEST_SERIAL_LEN + 1, "%s", serial_str); + return 1; +} + + +/* Scan TEMP_DIR and return one past the highest existing numbered run dir. */ +static int next_run_number(void) { + int max_num = 0; + DIR *dir = opendir(TEMP_DIR); + if (!dir) + return 1; + + struct dirent *ent; + while ((ent = readdir(dir)) != NULL) { + int num = 0; + if (strlen(ent->d_name) == TEST_SERIAL_LEN + && sscanf(ent->d_name, TEST_SERIAL_FMT, &num) == 1 + && num > max_num) + max_num = num; } + closedir(dir); + return max_num + 1; +} - /* Scan for the highest existing numbered run directory. */ - int run_num = 0; - DIR *d = opendir(TEMP_DIR); - if (d) { - struct dirent *ent; - while ((ent = readdir(d)) != NULL) { - int num = 0; - if (strlen(ent->d_name) == 6 - && sscanf(ent->d_name, "%6d", &num) == 1 - && num > run_num) - run_num = num; - } - closedir(d); + +/* + * Pioneer: create the run directory, write the serial to the coordination + * file, and update the "latest" symlink. Cleans up pgid_file on failure. + */ +static void pioneer_setup(int fd_create, const char *pgid_file) { + char serial_str[TEST_SERIAL_LEN + 1] = {0}; + + /* Reuse the previous run directory if it is still empty. */ + if (try_reuse_latest(serial_str)) { + (void)write(fd_create, serial_str, strlen(serial_str)); + close(fd_create); + return; } - /* Create the next sequential run directory (mkdir is atomic on POSIX). */ - for (run_num++; run_num < 1000000; run_num++) { - if (snprintf(run_dir_storage, sizeof(run_dir_storage), - TEMP_DIR "/%06d", run_num) >= (int)sizeof(run_dir_storage)) { - run_dir_storage[0] = '\0'; - close(fd_create); - unlink(pgid_file); - return; - } - if (mkdir(run_dir_storage, 0777) == 0) + /* Find and create the next sequential run directory. */ + for (int run_num = next_run_number(); run_num < TEST_SERIAL_MAX; run_num++) { + int n = snprintf(run_dir_storage, sizeof(run_dir_storage), + TEMP_DIR "/%06d", run_num); + if (n < 0 || n >= (int)sizeof(run_dir_storage)) break; - if (errno != EEXIST) { - run_dir_storage[0] = '\0'; + if (mkdir(run_dir_storage, 0777) == 0) { + snprintf(serial_str, sizeof(serial_str), "%06d", run_num); + (void)write(fd_create, serial_str, strlen(serial_str)); close(fd_create); - unlink(pgid_file); + char latest[PATH_MAX]; + snprintf(latest, sizeof(latest), TEMP_DIR "/latest"); + unlink(latest); + symlink(serial_str, latest); /* best-effort */ return; } + if (errno != EEXIST) + break; } - /* Write the serial into the coordination file and update "latest". */ - char serial_str[7]; - snprintf(serial_str, sizeof(serial_str), "%06d", run_num); - (void)write(fd_create, serial_str, strlen(serial_str)); + /* Failed to create a run directory; clean up and fall back. */ + run_dir_storage[0] = '\0'; close(fd_create); + unlink(pgid_file); +} + + +__attribute__((constructor)) +static void init_run_dir(void) { + ensure_tmp_dir(); + clean_stale_pgid_files(); + + pid_t pgid = getpgrp(); + char pgid_file[PATH_MAX]; + snprintf(pgid_file, sizeof(pgid_file), TEMP_DIR "/" PGID_FILE_TEMPLATE, (int)pgid); + + /* Follower: join an existing run for this PGID. */ + if (join_existing_run(pgid_file)) + return; + + /* Pioneer: atomically claim the coordination file. */ + int fd_create = open(pgid_file, O_WRONLY | O_CREAT | O_EXCL, 0600); + if (fd_create < 0) { + if (errno == EEXIST) + wait_for_pioneer(pgid_file); /* lost the race; become a follower */ + return; + } - char latest[PATH_MAX]; - snprintf(latest, sizeof(latest), TEMP_DIR "/latest"); - unlink(latest); - symlink(serial_str, latest); /* best-effort */ + pioneer_setup(fd_create, pgid_file); } From a1376dedf513b17fb7cb2743a4f56da6a06cfed3 Mon Sep 17 00:00:00 2001 From: "E. Madison Bray" <676149+embray@users.noreply.github.com> Date: Thu, 26 Mar 2026 16:10:14 +0100 Subject: [PATCH 5/5] build: fix distcheck test: use TEST_SERIAL_FMT macro in more places where it's relevant --- tests/Makefile.am | 1 + tests/util.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index d9d9a0cc..6eab7271 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -375,6 +375,7 @@ EXTRA_DIST += \ fixtures/scalars-out.asdf \ fixtures/tagged-scalars.asdf \ fixtures/tiles.asdf \ + fixtures/time.asdf \ fixtures/trivial-extension.asdf \ fixtures/value-types.asdf \ fixtures/verify-checksums/255-2-blocks.verify-checksums.txt \ diff --git a/tests/util.c b/tests/util.c index 33ff6bec..8a1aba8d 100644 --- a/tests/util.c +++ b/tests/util.c @@ -245,11 +245,11 @@ static void pioneer_setup(int fd_create, const char *pgid_file) { /* Find and create the next sequential run directory. */ for (int run_num = next_run_number(); run_num < TEST_SERIAL_MAX; run_num++) { int n = snprintf(run_dir_storage, sizeof(run_dir_storage), - TEMP_DIR "/%06d", run_num); + TEMP_DIR "/" TEST_SERIAL_FMT, run_num); if (n < 0 || n >= (int)sizeof(run_dir_storage)) break; if (mkdir(run_dir_storage, 0777) == 0) { - snprintf(serial_str, sizeof(serial_str), "%06d", run_num); + snprintf(serial_str, sizeof(serial_str), TEST_SERIAL_FMT, run_num); (void)write(fd_create, serial_str, strlen(serial_str)); close(fd_create); char latest[PATH_MAX];