Skip to content

Commit

Permalink
Incorrect validation for GNCAmountEdit
Browse files Browse the repository at this point in the history
gnc_amount_edit_expr_is_valid can return 0 for validation OK, 1 for an
error with no position information and also the position of that error.
If the error was at position 0, that will look like the validation was
OK so instead return 1 and use a GError parameter using the position as
the code and setting the message to the common format used. Also as
gnc_amount_edit_evaluate could also benefit from using the GError add
that as a parameter and initially set all occurrences of use to NULL.
  • Loading branch information
Bob-IT committed May 31, 2021
1 parent 788166b commit db6f233
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 31 deletions.
2 changes: 1 addition & 1 deletion gnucash/gnome-utils/dialog-account.c
Expand Up @@ -947,7 +947,7 @@ gnc_new_account_ok (AccountWindow *aw)
return;
}

if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (aw->opening_balance_edit)))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (aw->opening_balance_edit), NULL))
{
const char *message = _("You must enter a valid opening balance "
"or leave it blank.");
Expand Down
12 changes: 6 additions & 6 deletions gnucash/gnome-utils/dialog-transfer.c
Expand Up @@ -991,7 +991,7 @@ gnc_xfer_amount_update_cb(GtkWidget *widget, GdkEventFocus *event,
XferDialog * xferData = data;
g_return_val_if_fail (xferData != NULL, FALSE);

gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->amount_edit));
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->amount_edit), NULL);

gnc_xfer_update_to_amount (xferData);

Expand Down Expand Up @@ -1027,7 +1027,7 @@ gnc_xfer_update_to_amount (XferDialog *xferData)
scu = gnc_commodity_get_fraction(xferData->to_commodity);

/* Determine the amount to transfer. */
if (!gnc_amount_edit_evaluate(price_edit) ||
if (!gnc_amount_edit_evaluate(price_edit, NULL) ||
gnc_numeric_zero_p(price_value = gnc_amount_edit_get_amount(price_edit)))
to_amount = gnc_numeric_zero();
else
Expand Down Expand Up @@ -1075,7 +1075,7 @@ gnc_xfer_to_amount_update_cb(GtkWidget *widget, GdkEventFocus *event,
XferDialog *xferData = data;
gnc_numeric price_value;

gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->to_amount_edit));
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->to_amount_edit), NULL);
price_value = gnc_xfer_dialog_compute_price_value(xferData);
gnc_amount_edit_set_amount(GNC_AMOUNT_EDIT(xferData->price_edit),
price_value);
Expand Down Expand Up @@ -1474,7 +1474,7 @@ check_accounts (XferDialog* xferData, Account* from_account,
static gboolean
check_edit(XferDialog *xferData)
{
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->price_edit)))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->price_edit), NULL))
{
if (gtk_toggle_button_get_active
(GTK_TOGGLE_BUTTON(xferData->price_radio)))
Expand All @@ -1485,7 +1485,7 @@ check_edit(XferDialog *xferData)
}
}

if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->to_amount_edit)))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->to_amount_edit), NULL))
{
if (gtk_toggle_button_get_active
(GTK_TOGGLE_BUTTON(xferData->amount_radio)))
Expand Down Expand Up @@ -1708,7 +1708,7 @@ gnc_xfer_dialog_response_cb (GtkDialog *dialog, gint response, gpointer data)
!check_accounts(xferData, from_account, to_account))
return;

if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->amount_edit)))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (xferData->amount_edit), NULL))
{
gnc_parse_error_dialog (xferData, _("You must enter a valid amount."));
LEAVE("no amount");
Expand Down
52 changes: 42 additions & 10 deletions gnucash/gnome-utils/gnc-amount-edit.c
Expand Up @@ -28,6 +28,7 @@

#include <gtk/gtk.h>
#include <gdk/gdkkeysyms.h>
#include <glib/gi18n.h>

#include "gnc-amount-edit.h"
#include "gnc-exp-parser.h"
Expand Down Expand Up @@ -229,7 +230,7 @@ gnc_amount_edit_key_press (GtkWidget *widget, GdkEventKey *event)
return result;
}

gnc_amount_edit_evaluate (gae);
gnc_amount_edit_evaluate (gae, NULL);

return TRUE;
}
Expand All @@ -245,13 +246,21 @@ gnc_amount_edit_new (void)
return GTK_WIDGET(gae);
}

static inline GQuark
exp_validate_quark (void)
{
return g_quark_from_static_string ("exp_validate");
}

gint
gnc_amount_edit_expr_is_valid (GNCAmountEdit *gae, gnc_numeric *amount,
gboolean empty_ok)
gboolean empty_ok, GError **error)
{
const char *string;
char *error_loc;
gboolean ok;
gchar *err_msg = NULL;
gint err_code;

g_return_val_if_fail (gae != NULL, -1);
g_return_val_if_fail (GNC_IS_AMOUNT_EDIT(gae), -1);
Expand All @@ -274,25 +283,39 @@ gnc_amount_edit_expr_is_valid (GNCAmountEdit *gae, gnc_numeric *amount,

/* Not ok */
if (error_loc != NULL)
return error_loc - string;
{
err_code = error_loc - string;
err_msg = g_strdup_printf (_("An error occurred while processing '%s' at position %d"),
string, err_code);
}
else
return 1;
{
err_code = 1000;
err_msg = g_strdup_printf (_("An error occurred while processing '%s'"),
string);
}

if (error)
g_set_error_literal (error, exp_validate_quark(), err_code, err_msg);

g_free (err_msg);
return 1;
}

gboolean
gnc_amount_edit_evaluate (GNCAmountEdit *gae)
gnc_amount_edit_evaluate (GNCAmountEdit *gae, GError **error)
{
gint result;
gnc_numeric amount;
GError *tmp_error = NULL;

g_return_val_if_fail (gae != NULL, FALSE);
g_return_val_if_fail (GNC_IS_AMOUNT_EDIT(gae), FALSE);


if (!gae->need_to_parse)
return TRUE;

result = gnc_amount_edit_expr_is_valid (gae, &amount, FALSE);
result = gnc_amount_edit_expr_is_valid (gae, &amount, FALSE, &tmp_error);

if (result == -1) /* field was empty and may remain so */
return TRUE;
Expand All @@ -313,7 +336,16 @@ gnc_amount_edit_evaluate (GNCAmountEdit *gae)
}

/* Parse error */
gtk_editable_set_position (GTK_EDITABLE(gae), result);
if (tmp_error)
{
if (tmp_error->code < 1000)
gtk_editable_set_position (GTK_EDITABLE(gae), tmp_error->code);

if (error)
g_propagate_error (error, tmp_error);
else
g_error_free (tmp_error);
}
return FALSE;
}

Expand All @@ -323,7 +355,7 @@ gnc_amount_edit_get_amount (GNCAmountEdit *gae)
g_return_val_if_fail (gae != NULL, gnc_numeric_zero ());
g_return_val_if_fail (GNC_IS_AMOUNT_EDIT(gae), gnc_numeric_zero ());

gnc_amount_edit_evaluate (gae);
gnc_amount_edit_evaluate (gae, NULL);

return gae->amount;
}
Expand All @@ -334,7 +366,7 @@ gnc_amount_edit_get_damount (GNCAmountEdit *gae)
g_return_val_if_fail (gae != NULL, 0.0);
g_return_val_if_fail (GNC_IS_AMOUNT_EDIT(gae), 0.0);

gnc_amount_edit_evaluate (gae);
gnc_amount_edit_evaluate (gae, NULL);

return gnc_numeric_to_double (gae->amount);
}
Expand Down
13 changes: 9 additions & 4 deletions gnucash/gnome-utils/gnc-amount-edit.h
Expand Up @@ -135,23 +135,28 @@ double gnc_amount_edit_get_damount (GNCAmountEdit *gae);
* @amount: parameter to hold the value of the parsed expression
* @empty_ok: if true, an empty field is skipped, otherwise an empty field
* parses as 0
*
* @error: if error location information is available it will be stored
* in this variable. Set it to NULL if you don't want the error.
*
* If needed, parse the expression in the amount entry. If there's no
* parsing error, it returns the amount, otherwise it returns the
* position in the expression where the error occurred.
*
* Return * 0 if the parsing was successful (note that if !empty_ok,
* an empty field will parse to 0)
* * -1 if the field is empty and that's ok (empty_ok)
* * error position if there was a parsing error
* * 1 parsing failed
*/
gint gnc_amount_edit_expr_is_valid (GNCAmountEdit *gae,
gnc_numeric *amount,
gboolean empty_ok);
gboolean empty_ok,
GError **error);

/**
* gnc_amount_edit_evaluate
* @gae: The GNCAmountEdit widget
* @error: if error location information is available it will be stored
* in this variable. Set it to NULL if you don't want the error.
*
* If needed, parse the expression in the amount entry
* and replace the expression with the result of evaluation.
Expand All @@ -160,7 +165,7 @@ gint gnc_amount_edit_expr_is_valid (GNCAmountEdit *gae,
*
* Return TRUE if parsing was successful or there was no need to parse.
*/
gboolean gnc_amount_edit_evaluate (GNCAmountEdit *gae);
gboolean gnc_amount_edit_evaluate (GNCAmountEdit *gae, GError **error);

/**
* gnc_amount_edit_set_print_flags:
Expand Down
6 changes: 3 additions & 3 deletions gnucash/gnome/assistant-stock-split.c
Expand Up @@ -277,14 +277,14 @@ gnc_stock_split_assistant_details_complete (GtkAssistant *assistant,
gnc_numeric amount;
gint result;

result = gnc_amount_edit_expr_is_valid (GNC_AMOUNT_EDIT (info->distribution_edit), &amount, TRUE);
result = gnc_amount_edit_expr_is_valid (GNC_AMOUNT_EDIT (info->distribution_edit), &amount, TRUE, NULL);
if ( result != 0)
return FALSE; /* Parsing error or field is empty */

if (gnc_numeric_zero_p (amount))
return FALSE; /* field value is 0 */

result = gnc_amount_edit_expr_is_valid (GNC_AMOUNT_EDIT (info->price_edit), &amount, TRUE);
result = gnc_amount_edit_expr_is_valid (GNC_AMOUNT_EDIT (info->price_edit), &amount, TRUE, NULL);
if (result == -1)
return TRUE; /* Optional field is empty */
else if ( result > 0)
Expand All @@ -305,7 +305,7 @@ gnc_stock_split_assistant_cash_complete (GtkAssistant *assistant,
gint result;
Account *account;

result = gnc_amount_edit_expr_is_valid (GNC_AMOUNT_EDIT (info->cash_edit), &amount, TRUE);
result = gnc_amount_edit_expr_is_valid (GNC_AMOUNT_EDIT (info->cash_edit), &amount, TRUE, NULL);
if (result == -1)
return TRUE; /* Optional field is empty */
else if ( result > 0)
Expand Down
2 changes: 1 addition & 1 deletion gnucash/gnome/dialog-customer.c
Expand Up @@ -283,7 +283,7 @@ static gboolean check_edit_amount (GtkWidget *amount,
gnc_numeric *min, gnc_numeric *max,
const char * error_message)
{
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (amount)))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (amount), NULL))
{
if (error_message)
gnc_error_dialog (gnc_ui_get_gtk_window (amount), "%s", error_message);
Expand Down
2 changes: 1 addition & 1 deletion gnucash/gnome/dialog-fincalc.c
Expand Up @@ -350,7 +350,7 @@ can_calc_value(FinCalcDialog *fcd, FinCalcValue value, int *error_item)
/* treat PAYMENT_PERIODS as a plain GtkEntry */
if (i != PAYMENT_PERIODS)
{
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (fcd->amounts[i])))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (fcd->amounts[i]), NULL))
{
*error_item = i;
return bad_exp;
Expand Down
4 changes: 2 additions & 2 deletions gnucash/gnome/dialog-invoice.c
Expand Up @@ -1364,7 +1364,7 @@ static gboolean
gnc_invoice_window_leave_to_charge_cb (GtkWidget *widget, GdkEventFocus *event,
gpointer data)
{
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (widget));
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (widget), NULL);
return FALSE;
}

Expand Down Expand Up @@ -1761,7 +1761,7 @@ gnc_invoice_redraw_all_cb (GnucashRegister *g_reg, gpointer data)

if (iw->to_charge_edit)
{
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (iw->to_charge_edit));
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (iw->to_charge_edit), NULL);
to_charge_amt = gnc_amount_edit_get_amount(GNC_AMOUNT_EDIT(iw->to_charge_edit));
}

Expand Down
2 changes: 1 addition & 1 deletion gnucash/gnome/dialog-price-editor.c
Expand Up @@ -274,7 +274,7 @@ gui_to_price (PriceEditDialog *pedit_dialog)
type = type_index_to_string
(gtk_combo_box_get_active (GTK_COMBO_BOX (pedit_dialog->type_combobox)));

if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (pedit_dialog->price_edit)))
if (!gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (pedit_dialog->price_edit), NULL))
return _("You must enter a valid amount.");

value = gnc_amount_edit_get_amount
Expand Down
2 changes: 1 addition & 1 deletion gnucash/gnome/window-reconcile.c
Expand Up @@ -317,7 +317,7 @@ gnc_start_recn_update_cb(GtkWidget *widget, GdkEventFocus *event,
{
gnc_numeric value;

gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT(data->end_value));
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT(data->end_value), NULL);

value = gnc_amount_edit_get_amount (GNC_AMOUNT_EDIT(data->end_value));
data->user_set_value = !gnc_numeric_equal(value, data->original_value);
Expand Down
2 changes: 1 addition & 1 deletion gnucash/gnome/window-reconcile2.c
Expand Up @@ -307,7 +307,7 @@ gnc_start_recn2_update_cb (GtkWidget *widget, GdkEventFocus *event,
{
gnc_numeric value;

gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (data->end_value));
gnc_amount_edit_evaluate (GNC_AMOUNT_EDIT (data->end_value), NULL);

value = gnc_amount_edit_get_amount (GNC_AMOUNT_EDIT (data->end_value));
data->user_set_value = !gnc_numeric_equal (value, data->original_value);
Expand Down

0 comments on commit db6f233

Please sign in to comment.