Add a "hidden" array of UAT entry data to allow separation between UAT file syntax errors and "syntactically correct, but invalid field". Now UAT files load all entries into the "hidden" array (raw_data), but only adds valid ones to the user_data, which is used by the dissectors.

This is a start to fixing bug 7471 (https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7471) and is being committed to get the new ABI/API in before the 1.10 release.
What remains is the "GUI portion" (GTK+qt) to indicate to users which UAT entries are invalid.

svn path=/trunk/; revision=48960

6 files changed, 92 insertions, 23 deletions

diff --git a/airpcap_loader.c b/airpcap_loader.c
index bca9708011..fd7abdd284 100644
--- a/airpcap_loader.c
+++ b/airpcap_loader.c
@@ -279,7 +279,7 @@ set_wep_key(pref_t *pref, gpointer ud _U_)
 
             uat_key.string = get_key_string(new_key);
             uat_key.key = new_key->type;
-            uat_add_record(uat, &uat_key);
+            uat_add_record(uat, &uat_key, TRUE);
         }
 
         uat_save(uat, &err);
diff --git a/epan/uat-int.h b/epan/uat-int.h
index e079de256a..6097e47d6b 100644
--- a/epan/uat-int.h
+++ b/epan/uat-int.h
@@ -63,6 +63,8 @@ struct _uat_t {
     uat_field_t* fields;
     guint ncols;
     GArray* user_data;
+    GArray* raw_data;
+    GArray* valid_data;
     gboolean changed;
     uat_rep_t* rep;
     uat_rep_free_cb_t free_rep;
@@ -77,7 +79,7 @@ void uat_init(void);
 void uat_reset(void);
 
 WS_DLL_PUBLIC
-void* uat_add_record(uat_t*, const void* orig_rec_ptr);
+void* uat_add_record(uat_t*, const void* orig_rec_ptr, gboolean valid_rec);
 
 WS_DLL_PUBLIC
 void uat_swap(uat_t*, guint idx_a, guint idx_b);
@@ -96,7 +98,8 @@ gboolean uat_save(uat_t* , const char** );
 void uat_load_all(void);
 
 #define UAT_UPDATE(uat) do { *((uat)->user_ptr) = (void*)((uat)->user_data->data); *((uat)->nrows_p) = (uat)->user_data->len; } while(0)
-#define UAT_INDEX_PTR(uat,idx) (uat->user_data->data + (uat->record_size * (idx)))
+#define UAT_INDEX_PTR(uat,idx) (uat->raw_data->data + (uat->record_size * (idx)))
+#define UAT_USER_INDEX_PTR(uat,idx) (uat->user_data->data + (uat->record_size * (idx)))
 #endif
 
 /*
diff --git a/epan/uat.c b/epan/uat.c
index 87c90b27f1..ebe11c99b6 100644
--- a/epan/uat.c
+++ b/epan/uat.c
@@ -93,6 +93,8 @@ uat_t* uat_new(const char* name,
     uat->post_update_cb = post_update_cb;
     uat->fields = flds_array;
     uat->user_data = g_array_new(FALSE,FALSE,(guint)uat->record_size);
+    uat->raw_data = g_array_new(FALSE,FALSE,(guint)uat->record_size);
+    uat->valid_data = g_array_new(FALSE,FALSE,sizeof(gboolean));
     uat->changed = FALSE;
     uat->loaded = FALSE;
     uat->from_global = FALSE;
@@ -119,18 +121,37 @@ uat_t* uat_new(const char* name,
     return uat;
 }
 
-void* uat_add_record(uat_t* uat, const void* data) {
+void* uat_add_record(uat_t* uat, const void* data, gboolean valid_rec) {
     void* rec;
+    gboolean* valid;
 
-    g_array_append_vals (uat->user_data, data, 1);
+    /* Save a copy of the raw (possibly that may contain invalid field values) data */
+    g_array_append_vals (uat->raw_data, data, 1);
 
-    rec = uat->user_data->data + (uat->record_size * (uat->user_data->len-1));
+    rec = uat->raw_data->data + (uat->record_size * (uat->raw_data->len-1));
 
     if (uat->copy_cb) {
         uat->copy_cb(rec, data, (unsigned int) uat->record_size);
     }
 
-    UAT_UPDATE(uat);
+    if (valid_rec) {
+        /* Add a "known good" record to the list to be used by the dissector */
+        g_array_append_vals (uat->user_data, data, 1);
+
+        rec = uat->user_data->data + (uat->record_size * (uat->user_data->len-1));
+
+        if (uat->copy_cb) {
+            uat->copy_cb(rec, data, (unsigned int) uat->record_size);
+        }
+
+        UAT_UPDATE(uat);
+    } else {
+        rec = NULL;
+    }
+
+    g_array_append_vals (uat->valid_data, &valid_rec, 1);
+    valid = (gboolean*)(uat->valid_data->data + (sizeof(gboolean) * (uat->valid_data->len-1)));
+    *valid = valid_rec;
 
     return rec;
 }
@@ -138,8 +159,9 @@ void* uat_add_record(uat_t* uat, const void* data) {
 void uat_swap(uat_t* uat, guint a, guint b) {
     size_t s = uat->record_size;
     void* tmp = ep_alloc(s);
+    gboolean tmp_bool;
 
-    g_assert( a < uat->user_data->len && b < uat->user_data->len );
+    g_assert( a < uat->raw_data->len && b < uat->raw_data->len );
 
     if (a == b) return;
 
@@ -147,20 +169,23 @@ void uat_swap(uat_t* uat, guint a, guint b) {
     memcpy(UAT_INDEX_PTR(uat,a), UAT_INDEX_PTR(uat,b), s);
     memcpy(UAT_INDEX_PTR(uat,b), tmp, s);
 
+    tmp_bool = *(gboolean*)(uat->valid_data->data + (sizeof(gboolean) * (a)));
+    *(gboolean*)(uat->valid_data->data + (sizeof(gboolean) * (a))) = *(gboolean*)(uat->valid_data->data + (sizeof(gboolean) * (b)));
+    *(gboolean*)(uat->valid_data->data + (sizeof(gboolean) * (b))) = tmp_bool;
+
+
 }
 
 void uat_remove_record_idx(uat_t* uat, guint idx) {
 
-    g_assert( idx < uat->user_data->len );
+    g_assert( idx < uat->raw_data->len );
 
     if (uat->free_cb) {
         uat->free_cb(UAT_INDEX_PTR(uat,idx));
     }
 
-    g_array_remove_index(uat->user_data, idx);
-
-    UAT_UPDATE(uat);
-
+    g_array_remove_index(uat->raw_data, idx);
+    g_array_remove_index(uat->valid_data, idx);
 }
 
 /* The returned filename was g_malloc()'d so the caller must free it */
@@ -270,6 +295,34 @@ gboolean uat_save(uat_t* uat, const char** error) {
     *error = NULL;
     g_free (fname);
 
+    /* Ensure raw_data is synced with user_data and all "good" entries have been accounted for */
+
+    /* Start by clearing current user_data */
+    for ( i = 0 ; i < uat->user_data->len ; i++ ) {
+        if (uat->free_cb) {
+            uat->free_cb(UAT_USER_INDEX_PTR(uat,i));
+        }
+    }
+    g_array_set_size(uat->user_data,0);
+
+    *((uat)->user_ptr) = NULL;
+    *((uat)->nrows_p) = 0;
+
+    /* Now copy "good" raw_data entries to user_data */
+    for ( i = 0 ; i < uat->raw_data->len ; i++ ) {
+        void* rec = uat->raw_data->data + (uat->record_size * i);
+        gboolean* valid = (gboolean*)(uat->valid_data->data + sizeof(gboolean)*i);
+        if (*valid) {
+            g_array_append_vals(uat->user_data, rec, 1);
+            if (uat->copy_cb) {
+                uat->copy_cb(UAT_USER_INDEX_PTR(uat,i), rec, (unsigned int) uat->record_size);
+            }
+
+            UAT_UPDATE(uat);
+        }
+    }
+
+
     fprintf(fp,"# This file is automatically generated, DO NOT MODIFY.\n");
 
     for ( i = 0 ; i < uat->user_data->len ; i++ ) {
@@ -318,11 +371,19 @@ void uat_clear(uat_t* uat) {
 
     for ( i = 0 ; i < uat->user_data->len ; i++ ) {
         if (uat->free_cb) {
+            uat->free_cb(UAT_USER_INDEX_PTR(uat,i));
+        }
+    }
+
+    for ( i = 0 ; i < uat->raw_data->len ; i++ ) {
+        if (uat->free_cb) {
             uat->free_cb(UAT_INDEX_PTR(uat,i));
         }
     }
 
+    g_array_set_size(uat->raw_data,0);
     g_array_set_size(uat->user_data,0);
+    g_array_set_size(uat->valid_data,0);
 
     *((uat)->user_ptr) = NULL;
     *((uat)->nrows_p) = 0;
diff --git a/epan/uat_load.l b/epan/uat_load.l
index c306d4d14a..117bb3355c 100644
--- a/epan/uat_load.l
+++ b/epan/uat_load.l
@@ -71,6 +71,7 @@
 #endif
 
 static uat_t* uat;
+static gboolean valid_record;
 static guint colnum;
 static gchar* ptrx;
 static guint len;
@@ -86,7 +87,8 @@ static guint parse_str_pos;
 	{ const gchar* errx; \
 	if (uat->fields[colnum].cb.chk) { \
 		if ( ! uat->fields[colnum].cb.chk(record, ptrx, len, uat->fields[colnum].cbdata.chk, uat->fields[colnum].fld_data, &errx) ) { \
-			ERROR(("%s",errx)); \
+			error = ep_strdup_printf("%s:%d: %s",uat->filename,linenum,ep_strdup_printf("%s",errx)); \
+			valid_record = FALSE; \
 		}\
 	}\
 	uat->fields[colnum].cb.set(record, ptrx, len, uat->fields[colnum].cbdata.chk, uat->fields[colnum].fld_data);\
@@ -243,15 +245,16 @@ comment #[^\n]*\n
 
 	SET_FIELD();
 
-	rec = uat_add_record(uat, record);
+	rec = uat_add_record(uat, record, valid_record);
 
-	if (uat->update_cb)
+	if ((uat->update_cb) && (rec != NULL))
 		uat->update_cb(rec,&err);
 
 	if (err) {
 		ERROR(("%s",err));
 	}
 
+	valid_record = TRUE;
 	colnum = 0;
 	ptrx = NULL;
 	len = 0;
@@ -302,6 +305,7 @@ uat_load(uat_t *uat_in, const char **errx)
 	}
 
 	error = NULL;
+	valid_record = TRUE;
 	colnum = 0;
 	g_free(record);
 	record = g_malloc0(uat->record_size);
@@ -340,6 +344,7 @@ uat_load_str(uat_t *uat_in, char *entry, char **err)
 	yyin = NULL;
 
 	error = NULL;
+	valid_record = TRUE;
 	colnum = 0;
 	g_free(record);
 	record = g_malloc0(uat->record_size);
diff --git a/plugins/stats_tree/pinfo_stats_tree.c b/plugins/stats_tree/pinfo_stats_tree.c
index 3646f61181..143daf8d04 100644
--- a/plugins/stats_tree/pinfo_stats_tree.c
+++ b/plugins/stats_tree/pinfo_stats_tree.c
@@ -103,7 +103,7 @@ static void uat_plen_record_post_update_cb(void) {
 		for (i = 0; i < num_default; i++)
 		{
 			rec.packet_range = &default_range[i];
-			uat_add_record(plen_uat, &rec);
+			uat_add_record(plen_uat, &rec, TRUE);
 		}
 	}
 }
diff --git a/ui/gtk/uat_gui.c b/ui/gtk/uat_gui.c
index 6a3c91b9a4..f649bce5dd 100644
--- a/ui/gtk/uat_gui.c
+++ b/ui/gtk/uat_gui.c
@@ -127,7 +127,7 @@ static void set_buttons(uat_t *uat, gint row) {
 		gtk_widget_set_sensitive (uat->rep->bt_up, FALSE);
 	}
 
-	if (row < (gint)(*uat->nrows_p - 1) && row >= 0) {
+	if (row < (gint)(uat->raw_data->len - 1) && row >= 0) {
 		gtk_widget_set_sensitive (uat->rep->bt_down, TRUE);
 	} else {
 		gtk_widget_set_sensitive (uat->rep->bt_down, FALSE);
@@ -358,7 +358,7 @@ static gboolean uat_dlg_cb(GtkWidget *win _U_, gpointer user_data) {
 
 	if (dd->is_new) {
 		void *rec_tmp = dd->rec;
-		dd->rec = uat_add_record(dd->uat, dd->rec);
+		dd->rec = uat_add_record(dd->uat, dd->rec, TRUE);
 
 		if (dd->uat->free_cb) {
 			dd->uat->free_cb(rec_tmp);
@@ -372,7 +372,7 @@ static gboolean uat_dlg_cb(GtkWidget *win _U_, gpointer user_data) {
 	set_buttons(dd->uat, dd->uat->rep ? dd->uat->rep->selected : -1);
 
 	if (dd->is_new) {
-		append_row(dd->uat, (*dd->uat->nrows_p) - 1 );
+		append_row(dd->uat, dd->uat->raw_data->len - 1 );
 	} else {
 		reset_row(dd->uat, dd->row);
 	}
@@ -705,7 +705,7 @@ static void uat_down_cb(GtkButton *button _U_, gpointer u) {
 	uat_t *uat = (uat_t *)u;
 	gint   row = uat->rep->selected;
 
-	g_assert(row >= 0 && (guint) row < *uat->nrows_p - 1);
+	g_assert(row >= 0 && (guint) row < uat->raw_data->len - 1);
 
 	uat_swap(uat, row, row+1);
 	tree_view_list_store_move_selection(uat->rep->list, FALSE);
@@ -809,7 +809,7 @@ static void uat_refresh_cb(GtkButton *button _U_, gpointer u) {
 		report_failure("Error while loading %s: %s", uat->name, err);
 	}
 
-	for (i = 0 ; i < *(uat->nrows_p); i++) {
+	for (i = 0 ; i < uat->raw_data->len; i++) {
 		append_row(uat, i);
 	}
 }
@@ -987,7 +987,7 @@ static GtkWidget *uat_window(void *u) {
 			gtk_widget_set_tooltip_text(gtk_tree_view_column_get_button(column), f[colnum].desc);
 	}
 
-	for ( i = 0 ; i < *(uat->nrows_p); i++ ) {
+	for ( i = 0 ; i < uat->raw_data->len; i++ ) {
 		append_row(uat, i);
 	}