From f8caa028c9c1beed11297c9dc8b1c7cfb76b454c Mon Sep 17 00:00:00 2001 From: Allen Winter Date: Thu, 9 Jun 2022 12:41:41 -0400 Subject: Internally represent GEO properties as text This allows arbitrary precision for the GEO values fixes: #531 --- ReleaseNotes.txt | 1 + design-data/properties.csv | 2 +- src/libical-glib/api/i-cal-geo.xml | 32 +++++++++++++++++---------- src/libical/icalderivedvalue.c.in | 5 ++--- src/libical/icaltypes.h | 5 +++-- src/libical/icalvalue.c | 40 ++++++++++++++++----------------- src/test/regression.c | 45 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 93 insertions(+), 37 deletions(-) diff --git a/ReleaseNotes.txt b/ReleaseNotes.txt index 5a4c023b..898fac72 100644 --- a/ReleaseNotes.txt +++ b/ReleaseNotes.txt @@ -17,6 +17,7 @@ Version 3.1.0 (NOT RELEASED YET): before including * Allow previous recurrence iteration * Improved performance of recurrence iterators + * GEO property has arbitrary precision (values are internally stored as strings, not doubles) * All ical*_new_clone() functions have been deprecated in favour of ical*_clone() * Added support for Event Publishing (RFC 9073) and VALARM (RFC 9074) Extensions * icaltzutil_get_zone_directory() can use the TZDIR environment to find system zoneinfo diff --git a/design-data/properties.csv b/design-data/properties.csv index 7f019653..26867360 100644 --- a/design-data/properties.csv +++ b/design-data/properties.csv @@ -11,7 +11,7 @@ "CLASS","13","CLASS","CLASS" "COMMENT","15","TEXT","TEXT" "DESCRIPTION","29","TEXT","TEXT" -"GEO","39","GEO","FLOAT",is_structured +"GEO","39","GEO","TEXT",is_structured "LOCATION","43","TEXT","TEXT" "PERCENT-COMPLETE","54","INTEGER","INTEGER" "PRIORITY","56","INTEGER","INTEGER" diff --git a/src/libical-glib/api/i-cal-geo.xml b/src/libical-glib/api/i-cal-geo.xml index 7f81e20f..b7645184 100644 --- a/src/libical-glib/api/i-cal-geo.xml +++ b/src/libical-glib/api/i-cal-geo.xml @@ -10,8 +10,8 @@ Creates a new default #ICalGeo. struct icalgeotype geotype; - geotype.lat = 0; - geotype.lon = 0; + memset(geotype.lat, 0, ICAL_GEO_LEN); + memset(geotype.lon, 0, ICAL_GEO_LEN); return geotype; @@ -20,10 +20,14 @@ Creates a new #ICalGeo. struct icalgeotype geo; - + char dval[ICAL_GEO_LEN]; geo = i_cal_geo_new_default(); - geo.lat = lat; - geo.lon = lon; + memset(dval, 0, ICAL_GEO_LEN); + snprintf(dval, ICAL_GEO_LEN, "%lf", lat); + strncpy(geo.lat, dval, ICAL_GEO_LEN-1); + memset(dval, 0, ICAL_GEO_LEN); + snprintf(dval, ICAL_GEO_LEN, "%lf", lon); + strncpy(geo.lon, dval, ICAL_GEO_LEN-1); return i_cal_geo_new_full(geo); @@ -45,27 +49,33 @@ Gets the latitude of #ICalGeo. g_return_val_if_fail (geo != NULL, 0); - return ((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lat; + return atof(((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lat); Sets the latitude of #ICalGeo. - g_return_if_fail (geo != NULL && I_CAL_IS_GEO (geo)); - ((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lat = lat; + char dval[ICAL_GEO_LEN]; + g_return_if_fail (geo != NULL && I_CAL_IS_GEO (geo)); + memset(dval, 0, ICAL_GEO_LEN); + snprintf(dval, ICAL_GEO_LEN, "%lf", lat); + strncpy(((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lat, dval, ICAL_GEO_LEN-1); Gets the longitude of #ICalGeo. g_return_val_if_fail (geo != NULL, 0); - return ((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lon; + return atof(((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lon); Sets the longitude of #ICalGeo. - g_return_if_fail (geo != NULL && I_CAL_IS_GEO (geo)); - ((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lon = lon; + char dval[ICAL_GEO_LEN]; + g_return_if_fail (geo != NULL && I_CAL_IS_GEO (geo)); + memset(dval, 0, ICAL_GEO_LEN); + snprintf(dval, ICAL_GEO_LEN, "%lf", lon); + strncpy(((struct icalgeotype *)i_cal_object_get_native ((ICalObject *)geo))->lon, dval, ICAL_GEO_LEN-1); diff --git a/src/libical/icalderivedvalue.c.in b/src/libical/icalderivedvalue.c.in index 1b87c353..8dde33eb 100644 --- a/src/libical/icalderivedvalue.c.in +++ b/src/libical/icalderivedvalue.c.in @@ -430,9 +430,8 @@ void icalvalue_set_geo(icalvalue *value, struct icalgeotype v) struct icalgeotype icalvalue_get_geo(const icalvalue *value) { struct icalgeotype gt; - - gt.lat = 255.0; - gt.lon = 255.0; + strcpy(gt.lat, "255.0"); + strcpy(gt.lon, "255.0"); icalerror_check_arg_rx((value != 0), "value", gt); icalerror_check_value_type(value, ICAL_GEO_VALUE); diff --git a/src/libical/icaltypes.h b/src/libical/icaltypes.h index d606bc37..bad766b5 100644 --- a/src/libical/icaltypes.h +++ b/src/libical/icaltypes.h @@ -22,10 +22,11 @@ struct icaldatetimeperiodtype struct icalperiodtype period; }; +#define ICAL_GEO_LEN 16 struct icalgeotype { - double lat; - double lon; + char lat[ICAL_GEO_LEN]; + char lon[ICAL_GEO_LEN]; }; struct icaltriggertype diff --git a/src/libical/icalvalue.c b/src/libical/icalvalue.c index 6a15d788..c41a69a3 100644 --- a/src/libical/icalvalue.c +++ b/src/libical/icalvalue.c @@ -357,23 +357,20 @@ static icalvalue *icalvalue_new_enum(icalvalue_kind kind, int x_type, const char } /** - * Transforms a simple float number string into a double. + * Extracts a simple floating point number as a substring. * The decimal separator (if any) of the double has to be '.' * The code is locale *independent* and does *not* change the locale. * It should be thread safe. - * If you want a code that does the same job with a decimal separator - * dependent on the current locale, then use strtof() from libc. */ -static int simple_str_to_double(const char *from, double *result, char **to) +static int simple_str_to_doublestr(const char *from, char *result, char **to) { -#define TMP_NUM_SIZE 100 char *start = NULL, *end = NULL, *cur = (char *)from; - char tmp_buf[TMP_NUM_SIZE + 1]; /*hack */ #if !defined(HAVE_GETNUMBERFORMAT) struct lconv *loc_data = localeconv(); #endif int i = 0; + double dtest; /*sanity checks */ if (!from || !result) { @@ -385,8 +382,7 @@ static int simple_str_to_double(const char *from, double *result, char **to) cur++; start = cur; - /* copy the part that looks like a double into tmp_buf - * so that we can call strtof() on it. + /* copy the part that looks like a double into result. * during the copy, we give ourselves a chance to convert the '.' * into the decimal separator of the current locale. */ @@ -398,7 +394,6 @@ static int simple_str_to_double(const char *from, double *result, char **to) /*huh hoh, number is too big. getting out */ return 1; } - memset(tmp_buf, 0, TMP_NUM_SIZE + 1); /* copy the float number string into tmp_buf, and take * care to have the (optional) decimal separator be the one @@ -409,18 +404,22 @@ static int simple_str_to_double(const char *from, double *result, char **to) if (start[i] == '.' && loc_data && loc_data->decimal_point && loc_data->decimal_point[0] && loc_data->decimal_point[0] != '.') { /*replace '.' by the digit separator of the current locale */ - tmp_buf[i] = loc_data->decimal_point[0]; + result[i] = loc_data->decimal_point[0]; } else { - tmp_buf[i] = start[i]; + result[i] = start[i]; } } #else - GetNumberFormat(LOCALE_SYSTEM_DEFAULT, 0, start, NULL, tmp_buf, TMP_NUM_SIZE); + GetNumberFormat(LOCALE_SYSTEM_DEFAULT, 0, start, NULL, result, TMP_NUM_SIZE); #endif if (to) { *to = end; } - *result = atof(tmp_buf); + + /* now try to convert to a floating point number, to check for validity only */ + if (sscanf(result, "%lf", &dtest) != 1) { + return 1; + } return 0; } @@ -580,13 +579,14 @@ static icalvalue *icalvalue_new_from_string_with_error(icalvalue_kind kind, case ICAL_GEO_VALUE: { char *cur = NULL; - struct icalgeotype geo = { 0.0, 0.0 }; + struct icalgeotype geo; + memset(geo.lat, 0, ICAL_GEO_LEN); + memset(geo.lon, 0, ICAL_GEO_LEN); - if (simple_str_to_double(str, &geo.lat, &cur)) { + if (simple_str_to_doublestr(str, geo.lat, &cur)) { goto geo_parsing_error; } - - /*skip white spaces */ + /* skip white spaces */ while (cur && isspace((int)*cur)) { ++cur; } @@ -598,12 +598,12 @@ static icalvalue *icalvalue_new_from_string_with_error(icalvalue_kind kind, ++cur; - /*skip white spaces */ + /* skip white spaces */ while (cur && isspace((int)*cur)) { ++cur; } - if (simple_str_to_double(cur, &geo.lon, &cur)) { + if (simple_str_to_doublestr(cur, geo.lon, &cur)) { goto geo_parsing_error; } value = icalvalue_new_geo(geo); @@ -1102,7 +1102,7 @@ static char *icalvalue_geo_as_ical_string_r(const icalvalue *value) str = (char *)icalmemory_new_buffer(80); - snprintf(str, 80, "%f;%f", data.lat, data.lon); + snprintf(str, 80, "%s;%s", data.lat, data.lon); /* restore saved locale */ (void)setlocale(LC_NUMERIC, old_locale); diff --git a/src/test/regression.c b/src/test/regression.c index d52474f0..da3ba9af 100644 --- a/src/test/regression.c +++ b/src/test/regression.c @@ -4377,6 +4377,50 @@ void test_comma_in_quoted_value(void) icalcomponent_free(c); } +void test_geo_props(void) +{ + int estate; + icalcomponent *c; + icalproperty *p; + + c = icalparser_parse_string("BEGIN:VEVENT\n" "GEO:49.42612;7.75473\n" "END:VEVENT\n"); + ok("icalparser_parse_string()", (c != NULL)); + if (!c) { + exit(EXIT_FAILURE); + } + if (VERBOSE) + printf("%s", icalcomponent_as_ical_string(c)); + p = icalcomponent_get_first_property(c, ICAL_GEO_PROPERTY); + str_is("icalproperty_get_value_as_string() works", + icalproperty_get_value_as_string(p), "49.42612;7.75473"); + icalcomponent_free(c); + + c = icalparser_parse_string("BEGIN:VEVENT\n" "GEO:-0;+0\n" "END:VEVENT\n"); + ok("icalparser_parse_string()", (c != NULL)); + if (!c) { + exit(EXIT_FAILURE); + } + if (VERBOSE) + printf("%s", icalcomponent_as_ical_string(c)); + p = icalcomponent_get_first_property(c, ICAL_GEO_PROPERTY); + str_is("icalproperty_get_value_as_string() works", + icalproperty_get_value_as_string(p), "-0;+0"); + icalcomponent_free(c); + + estate = icalerror_get_errors_are_fatal(); + icalerror_set_errors_are_fatal(0); + c = icalparser_parse_string("BEGIN:VEVENT\n" "GEO:-0a;+0\n" "END:VEVENT\n"); + if (!c) { + exit(EXIT_FAILURE); + } + if (VERBOSE) + printf("%s", icalcomponent_as_ical_string(c)); + p = icalcomponent_get_first_property(c, ICAL_GEO_PROPERTY); + ok("expected fail icalcomponent_get_first_property()", (p == NULL)); + icalcomponent_free(c); + icalerror_set_errors_are_fatal(estate); +} + void test_zoneinfo_stuff(void) { #if defined(HAVE_SETENV) @@ -5337,6 +5381,7 @@ int main(int argc, char *argv[]) test_run("Test implicit DTEND and DURATION for VEVENT and VTODO", test_implicit_dtend_duration, do_test, do_header); test_run("Test icalvalue resets timezone on set", test_icalvalue_resets_timezone_on_set, do_test, do_header); test_run("Test removing TZID from DUE with icalcomponent_set_due", test_remove_tzid_from_due, do_test, do_header); + test_run("Test geo precision", test_geo_props, do_test, do_header); /** OPTIONAL TESTS go here... **/ -- cgit v1.2.1