gnucash maint: Bug 728722 - Setting number format details appear wrong in Help, section 10.3.4. Counters Book Options Tab
Geert Janssens
gjanssens at code.gnucash.org
Thu Mar 17 16:43:27 EDT 2016
Updated via https://github.com/Gnucash/gnucash/commit/a27abf76 (commit)
from https://github.com/Gnucash/gnucash/commit/8117a7c1 (commit)
commit a27abf766a1c0148a497d0f8000da92f0228398e
Author: Geert Janssens <janssens-geert at telenet.be>
Date: Thu Mar 17 22:37:15 2016 +0100
Bug 728722 - Setting number format details appear wrong in Help, section 10.3.4. Counters Book Options Tab
This is a follow-up commit to fix the core of the issue.
With this commit gnucash is more liberal at accepting
counter formats. It will accept either li, lli, I64i and
whatever is defined for G_GINT_64 or PRIx64 on the user's
platform. Internally the code will always convert the
specifier set by the user with PRIx64, which should always
be the correct one on any platform.
Additionally a few extra tests were added to stress the
counter format code a bit more.
diff --git a/src/libqof/qof/qofbook-p.h b/src/libqof/qof/qofbook-p.h
index 4560c83..393b2b0 100644
--- a/src/libqof/qof/qofbook-p.h
+++ b/src/libqof/qof/qofbook-p.h
@@ -61,13 +61,16 @@ backends (when reading the GncGUID from the data source). */
#define qof_book_set_guid(book,guid) \
qof_instance_set_guid(QOF_INSTANCE(book), guid)
-/** Validate a counter format string with the given
- * G_GINT64_FORMAT. Returns an error message if the format string
- * was invalid, or NULL if it is ok. The caller should free the
- * error message with g_free.
+/** Validate a counter format string with a given format specifier.
+ * If valid, returns a normalized format string,
+ * that is whatever long int specifier was used will be replaced with the value of
+ * the posix "PRIx64" macro.
+ * If not valid returns NULL and optionally set an error message is a non-null
+ * err_msg parameter was passed.
+ * The caller should free the returned format string and error message with g_free.
*/
-gchar *qof_book_validate_counter_format_internal(const gchar *p,
- const gchar* gint64_format);
+gchar *qof_book_normalize_counter_format_internal(const gchar *p,
+ const gchar* gint64_format, gchar **err_msg);
/** This debugging function can be used to traverse the book structure
* and all subsidiary structures, printing out which structures
diff --git a/src/libqof/qof/qofbook.c b/src/libqof/qof/qofbook.c
index e2b6aa2..c789ada 100644
--- a/src/libqof/qof/qofbook.c
+++ b/src/libqof/qof/qofbook.c
@@ -39,6 +39,7 @@
#include <string.h>
#include <glib.h>
+#include <inttypes.h>
#include "qof.h"
#include "qofevent-p.h"
@@ -407,6 +408,7 @@ qof_book_increment_and_format_counter (QofBook *book, const char *counter_name)
KvpValue *value;
gint64 counter;
gchar* format;
+ gchar* result;
if (!book)
{
@@ -456,16 +458,19 @@ qof_book_increment_and_format_counter (QofBook *book, const char *counter_name)
}
/* Generate a string version of the counter */
- return g_strdup_printf(format, counter);
+ result = g_strdup_printf(format, counter);
+ g_free (format);
+ return result;
}
gchar *
qof_book_get_counter_format(const QofBook *book, const char *counter_name)
{
KvpFrame *kvp;
- gchar *format;
+ gchar *user_format = NULL;
+ gchar *norm_format = NULL;
KvpValue *value;
- gchar *error;
+ gchar *error = NULL;
if (!book)
{
@@ -488,49 +493,75 @@ qof_book_get_counter_format(const QofBook *book, const char *counter_name)
return NULL;
}
- format = NULL;
-
/* Get the format string */
value = kvp_frame_get_slot_path (kvp, "counter_formats", counter_name, NULL);
if (value)
{
- format = kvp_value_get_string (value);
- error = qof_book_validate_counter_format(format);
- if (error != NULL)
+ user_format = kvp_value_get_string (value);
+ norm_format = qof_book_normalize_counter_format(user_format, &error);
+ if (!norm_format)
{
- PWARN("Invalid counter format string. Format string: '%s' Counter: '%s' Error: '%s')", format, counter_name, error);
+ PWARN("Invalid counter format string. Format string: '%s' Counter: '%s' Error: '%s')", user_format, counter_name, error);
/* Invalid format string */
- format = NULL;
+ user_format = NULL;
g_free(error);
}
}
/* If no (valid) format string was found, use the default format
* string */
- if (!format)
+ if (!norm_format)
{
/* Use the default format */
- format = "%.6" G_GINT64_FORMAT;
+ norm_format = g_strdup ("%.6" PRIx64);
}
- return format;
+ return norm_format;
}
gchar *
-qof_book_validate_counter_format(const gchar *p)
-{
- return qof_book_validate_counter_format_internal(p, G_GINT64_FORMAT);
+qof_book_normalize_counter_format(const gchar *p, gchar **err_msg)
+{
+ const gchar *valid_formats [] = {
+ G_GINT64_FORMAT,
+ "lli",
+ "I64i",
+ PRIx64,
+ "li",
+ NULL,
+ };
+ int i = 0;
+ gchar *normalized_spec = NULL;
+
+ while (valid_formats[i])
+ {
+
+ if (err_msg && *err_msg)
+ {
+ g_free (*err_msg);
+ *err_msg = NULL;
+ }
+
+ normalized_spec = qof_book_normalize_counter_format_internal(p, valid_formats[i], err_msg);
+ if (normalized_spec)
+ return normalized_spec; /* Found a valid format specifier, return */
+ i++;
+ }
+
+ return NULL;
}
gchar *
-qof_book_validate_counter_format_internal(const gchar *p,
- const gchar *gint64_format)
+qof_book_normalize_counter_format_internal(const gchar *p,
+ const gchar *gint64_format, gchar **err_msg)
{
- const gchar *conv_start, *tmp = NULL;
+ const gchar *conv_start, *base, *tmp = NULL;
+ gchar *normalized_str = NULL, *aux_str = NULL;
/* Validate a counter format. This is a very simple "parser" that
* simply checks for a single gint64 conversion specification,
* allowing all modifiers and flags that printf(3) specifies (except
* for the * width and precision, which need an extra argument). */
+ base = p;
/* Skip a prefix of any character except % */
while (*p)
@@ -551,7 +582,11 @@ qof_book_validate_counter_format_internal(const gchar *p,
}
if (!*p)
- return g_strdup("Format string ended without any conversion specification");
+ {
+ if (err_msg)
+ *err_msg = g_strdup("Format string ended without any conversion specification");
+ return NULL;
+ }
/* Store the start of the conversion for error messages */
conv_start = p;
@@ -563,6 +598,13 @@ qof_book_validate_counter_format_internal(const gchar *p,
* specification (e.g. "li" on Unix, "I64i" on Windows). */
tmp = strstr(p, gint64_format);
+ if (!tmp)
+ {
+ if (err_msg)
+ *err_msg = g_strdup_printf("Format string doesn't contain requested format specifier: %s", gint64_format);
+ return NULL;
+ }
+
/* Skip any number of flag characters */
while (*p && (tmp != p) && strchr("#0- +'I", *p))
{
@@ -570,39 +612,45 @@ qof_book_validate_counter_format_internal(const gchar *p,
tmp = strstr(p, gint64_format);
}
- /* Skip any number of field width digits */
- while (*p && (tmp != p) && strchr("0123456789", *p))
+ /* Skip any number of field width digits,
+ * and precision specifier digits (including the leading dot) */
+ while (*p && (tmp != p) && strchr("0123456789.", *p))
{
p++;
tmp = strstr(p, gint64_format);
}
- /* A precision specifier always starts with a dot */
- if (*p && *p == '.')
+ if (!*p)
{
- /* Skip the . */
- p++;
- /* Skip any number of precision digits */
- while (*p && strchr("0123456789", *p)) p++;
+ if (err_msg)
+ *err_msg = g_strdup_printf("Format string ended during the conversion specification. Conversion seen so far: %s", conv_start);
+ return NULL;
}
- if (!*p)
- return g_strdup_printf("Format string ended during the conversion specification. Conversion seen so far: %s", conv_start);
-
/* See if the format string starts with the correct format
* specification. */
tmp = strstr(p, gint64_format);
if (tmp == NULL)
{
- return g_strdup_printf("Invalid length modifier and/or conversion specifier ('%.4s'), it should be: %s", p, gint64_format);
+ if (err_msg)
+ *err_msg = g_strdup_printf("Invalid length modifier and/or conversion specifier ('%.4s'), it should be: %s", p, gint64_format);
+ return NULL;
}
else if (tmp != p)
{
- return g_strdup_printf("Garbage before length modifier and/or conversion specifier: '%*s'", (int)(tmp - p), p);
+ if (err_msg)
+ *err_msg = g_strdup_printf("Garbage before length modifier and/or conversion specifier: '%*s'", (int)(tmp - p), p);
+ return NULL;
}
+ /* Copy the string we have so far and add normalized format specifier for long int */
+ aux_str = g_strndup (base, p - base);
+ normalized_str = g_strconcat (aux_str, PRIx64, NULL);
+ g_free (aux_str);
+
/* Skip length modifier / conversion specifier */
p += strlen(gint64_format);
+ tmp = p;
/* Skip a suffix of any character except % */
while (*p)
@@ -617,14 +665,24 @@ qof_book_validate_counter_format_internal(const gchar *p,
/* Break on a single percent mark, which is the start of the
* conversion specification */
if (*p == '%')
- return g_strdup_printf("Format string contains unescaped %% signs (or multiple conversion specifications) at '%s'", p);
+ {
+ if (err_msg)
+ *err_msg = g_strdup_printf("Format string contains unescaped %% signs (or multiple conversion specifications) at '%s'", p);
+ g_free (normalized_str);
+ return NULL;
+ }
/* Skip all other characters */
p++;
}
+ /* Add the suffix to our normalized string */
+ aux_str = normalized_str;
+ normalized_str = g_strconcat (aux_str, tmp, NULL);
+ g_free (aux_str);
+
/* If we end up here, the string was valid, so return no error
* message */
- return NULL;
+ return normalized_str;
}
/* Determine whether this book uses trading accounts */
diff --git a/src/libqof/qof/qofbook.h b/src/libqof/qof/qofbook.h
index fef6d92..ab9a0db 100644
--- a/src/libqof/qof/qofbook.h
+++ b/src/libqof/qof/qofbook.h
@@ -322,11 +322,14 @@ gint64 qof_book_get_counter (QofBook *book, const char *counter_name);
*/
gchar *qof_book_increment_and_format_counter (QofBook *book, const char *counter_name);
-/** Validate a counter format string. Returns an error message if the
- * format string was invalid, or NULL if it is ok. The caller should
- * free the error message with g_free.
+/** Validate a counter format string. If valid, returns a normalized format string,
+ * that is whatever long int specifier was used will be replaced with the value of
+ * the posix "PRIx64" macro.
+ * If not valid returns NULL and optionally set an error message is a non-null
+ * err_msg parameter was passed.
+ * The caller should free the returned format string and error message with g_free.
*/
-gchar * qof_book_validate_counter_format(const gchar *format);
+gchar * qof_book_normalize_counter_format(const gchar *format, gchar **err_msg);
/** Get the format string to use for the named counter.
* The return value is NULL on error or the format string of the
diff --git a/src/libqof/qof/test/test-qofbook.c b/src/libqof/qof/test/test-qofbook.c
index ae53b28..c11876c 100644
--- a/src/libqof/qof/test/test-qofbook.c
+++ b/src/libqof/qof/test/test-qofbook.c
@@ -22,6 +22,7 @@
#include "config.h"
#include <string.h>
#include <glib.h>
+#include <inttypes.h>
#include <unittest-support.h>
#include "../qof.h"
#include "../qofbook-p.h"
@@ -123,64 +124,133 @@ test_book_readonly( Fixture *fixture, gconstpointer pData )
g_assert( qof_book_is_readonly( fixture->book ) );
}
static void
-test_book_validate_counter( void )
+test_book_normalize_counter( void )
{
- gchar *r;
+ gchar *r, *err_msg = NULL;
g_test_bug("644036");
+ g_test_bug("728722");
/* Test for detection of missing format conversion */
- r = qof_book_validate_counter_format("This string is missing the conversion specifier");
- g_assert(r);
- if (r && g_test_verbose())
+ r = qof_book_normalize_counter_format("This string is missing the conversion specifier", &err_msg);
+ g_assert(!r);
+ g_assert(err_msg);
+ if (!r && g_test_verbose())
{
- g_test_message("Counter format validation correctly failed: %s", r);
+ g_test_message("Counter format normalization correctly failed: %s", err_msg);
}
- g_free(r);
+ g_free(err_msg);
+ err_msg = NULL;
/* Test the usual Linux/Unix G_GINT64_FORMAT */
- r = qof_book_validate_counter_format_internal("Test - %li", "li");
- if (r && g_test_verbose())
+ r = qof_book_normalize_counter_format("Test - %li", &err_msg);
+ if (!r && g_test_verbose())
{
- g_test_message("Counter format validation erroneously failed: %s", r);
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
}
- g_assert(r == NULL);
+ g_assert_cmpstr( r, == , "Test - %" PRIx64);
+ g_assert(err_msg == NULL);
g_free(r);
/* Test the Windows G_GINT64_FORMAT */
- r = qof_book_validate_counter_format_internal("Test - %I64i", "I64i");
- if (r && g_test_verbose())
+ r = qof_book_normalize_counter_format("Test - %I64i", &err_msg);
+ if (!r && g_test_verbose())
{
- g_test_message("Counter format validation erroneously failed: %s", r);
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
}
- g_assert(r == NULL);
+ g_assert_cmpstr( r, == , "Test - %" PRIx64);
+ g_assert(err_msg == NULL);
g_free(r);
- /* Test the system's GINT64_FORMAT */
- r = qof_book_validate_counter_format("Test - %" G_GINT64_FORMAT);
- if (r && g_test_verbose())
+ /* Test the system's G_INT64_FORMAT */
+ r = qof_book_normalize_counter_format("Test - %" G_GINT64_FORMAT, &err_msg);
+ if (!r && g_test_verbose())
{
- g_test_message("Counter format validation erroneously failed: %s", r);
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
}
- g_assert(r == NULL);
+ g_assert_cmpstr( r, == , "Test - %" PRIx64);
+ g_assert(err_msg == NULL);
g_free(r);
- /* Test an erroneous Windows G_GINT64_FORMAT */
- r = qof_book_validate_counter_format_internal("Test - %li", "I64i");
- if (r && g_test_verbose())
+ /* Test the posix' PRIx64 */
+ r = qof_book_normalize_counter_format("Test - %" PRIx64, &err_msg);
+ if (!r && g_test_verbose())
{
- g_test_message("Counter format validation correctly failed: %s", r);
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
}
- g_assert(r);
+ g_assert_cmpstr( r, == , "Test - %" PRIx64);
+ g_assert(err_msg == NULL);
g_free(r);
- /* Test an erroneous Linux G_GINT64_FORMAT */
- r = qof_book_validate_counter_format_internal("Test - %I64i", "li");
- if (r && g_test_verbose())
+ /* Test the posix' PRIx64 with precision field */
+ r = qof_book_normalize_counter_format("Test - %.3" PRIx64, &err_msg);
+ if (!r && g_test_verbose())
+ {
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
+ }
+ g_assert_cmpstr( r, == , "Test - %.3" PRIx64);
+ g_assert(err_msg == NULL);
+ g_free(r);
+
+ /* Test the posix' PRIx64 with width field */
+ r = qof_book_normalize_counter_format("Test - %5" PRIx64, &err_msg);
+ if (!r && g_test_verbose())
+ {
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
+ }
+ g_assert_cmpstr( r, == , "Test - %5" PRIx64);
+ g_assert(err_msg == NULL);
+ g_free(r);
+
+ /* Test the posix' PRIx64 with width and precision field */
+ r = qof_book_normalize_counter_format("Test - %5.4" PRIx64, &err_msg);
+ if (!r && g_test_verbose())
+ {
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
+ }
+ g_assert_cmpstr( r, == , "Test - %5.4" PRIx64);
+ g_assert(err_msg == NULL);
+ g_free(r);
+
+ /* Test the usual Linux/Unix G_GINT64_FORMAT */
+ r = qof_book_normalize_counter_format_internal("Test - %li", "li", &err_msg);
+ if (!r && g_test_verbose())
+ {
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
+ }
+ g_assert_cmpstr( r, == , "Test - %" PRIx64);
+ g_assert(err_msg == NULL);
+ g_free(r);
+
+ /* Test the Windows G_GINT64_FORMAT */
+ r = qof_book_normalize_counter_format_internal("Test - %I64i", "I64i", &err_msg);
+ if (!r && g_test_verbose())
{
- g_test_message("Counter format validation correctly failed: %s", r);
+ g_test_message("Counter format normalization erroneously failed: %s", err_msg);
}
- g_assert(r);
+ g_assert_cmpstr( r, == , "Test - %" PRIx64);
+ g_assert(err_msg == NULL);
g_free(r);
+
+ /* Test an erroneous Windows G_GINT64_FORMAT */
+ r = qof_book_normalize_counter_format_internal("Test - %li", "I64i", &err_msg);
+ g_assert(!r);
+ g_assert(err_msg);
+ if (!r && g_test_verbose())
+ {
+ g_test_message("Counter format normalization correctly failed: %s", err_msg);
+ }
+ g_free(err_msg);
+ err_msg = NULL;
+
+ /* Test an erroneous Linux G_GINT64_FORMAT */
+ r = qof_book_normalize_counter_format_internal("Test - %I64i", "li", &err_msg);
+ g_assert(!r);
+ g_assert(err_msg);
+ if (!r && g_test_verbose())
+ {
+ g_test_message("Counter format normalization correctly failed: %s", err_msg);
+ }
+ g_free(err_msg);
}
static void
@@ -289,18 +359,18 @@ test_book_get_counter_format ( Fixture *fixture, gconstpointer pData )
g_free( test_struct.msg );
g_test_message( "Testing counter format when counter name is empty string" );
- r = qof_book_get_counter_format( fixture->book, NULL );
+ r = qof_book_get_counter_format( fixture->book, "" );
g_assert_cmpstr( r, == , NULL );
g_assert( g_strrstr( test_struct.msg, err_invalid_cnt ) != NULL );
g_free( test_struct.msg );
g_test_message( "Testing counter format with existing counter" );
r = qof_book_get_counter_format( fixture->book, counter_name );
- g_assert_cmpstr( r, == , "%.6" G_GINT64_FORMAT);
+ g_assert_cmpstr( r, == , "%.6" PRIx64);
g_test_message( "Testing counter format for default value" );
r = qof_book_get_counter_format( fixture->book, counter_name );
- g_assert_cmpstr( r, == , "%.6" G_GINT64_FORMAT);
+ g_assert_cmpstr( r, == , "%.6" PRIx64);
}
static void
@@ -331,7 +401,7 @@ test_book_increment_and_format_counter ( Fixture *fixture, gconstpointer pData )
g_free( test_struct.msg );
g_test_message( "Testing increment and format when counter name is empty string" );
- r = qof_book_increment_and_format_counter( fixture->book, NULL );
+ r = qof_book_increment_and_format_counter( fixture->book, "" );
g_assert_cmpstr( r, == , NULL );
g_free( r );
g_assert( g_strrstr( test_struct.msg, err_invalid_cnt ) != NULL );
@@ -757,7 +827,7 @@ void
test_suite_qofbook ( void )
{
GNC_TEST_ADD( suitename, "readonly", Fixture, NULL, setup, test_book_readonly, teardown );
- GNC_TEST_ADD_FUNC( suitename, "validate counter", test_book_validate_counter );
+ GNC_TEST_ADD_FUNC( suitename, "validate counter", test_book_normalize_counter );
GNC_TEST_ADD( suitename, "get string option", Fixture, NULL, setup, test_book_get_string_option, teardown );
GNC_TEST_ADD( suitename, "set string option", Fixture, NULL, setup, test_book_set_string_option, teardown );
GNC_TEST_ADD( suitename, "session not saved", Fixture, NULL, setup, test_book_session_not_saved, teardown );
Summary of changes:
src/libqof/qof/qofbook-p.h | 15 ++--
src/libqof/qof/qofbook.c | 128 +++++++++++++++++++++++----------
src/libqof/qof/qofbook.h | 11 +--
src/libqof/qof/test/test-qofbook.c | 140 +++++++++++++++++++++++++++----------
4 files changed, 214 insertions(+), 80 deletions(-)
More information about the gnucash-changes
mailing list