aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTomasz Moń <desowin@gmail.com>2022-08-07 12:31:48 +0200
committerTomasz Moń <desowin@gmail.com>2022-08-10 06:18:15 +0200
commitc1861ad1cc5ea673ee373e0cb69bfcd638ef06a2 (patch)
treeb25877413c4bbe462bd0040c9a2a77060308399c
parent86c6509cf32ce9350ed20dea4c741052ec65f3dc (diff)
extcap: Close capture session after extcap finishes
Wait up to 30 seconds for extcap process to finish after closing pipes. The wait is achieved in non-blocking fashion, i.e. the UI is completely responsive during the wait. Only actions related to capture process like capture control, file open, save, export are inactive during the wait. On Windows extcap child watch callback gets called immediately as the process is forcefully terminated. Prior to this change the extcap was forcefully terminated on Windows anyway. The wait is possible on UNIX systems if extcap does handle SIGPIPE and SIGTERM signals. The defaults handlers for SIGPIPE and SIGTERM simply terminate the process so for large number of extcaps there is no change. If extcap does not finish within 30 seconds, it is forcefully terminated using SIGKILL signal.
-rw-r--r--CMakeLists.txt4
-rw-r--r--capture/capture_session.h2
-rw-r--r--capture/capture_sync.c60
-rw-r--r--capture_opts.c19
-rw-r--r--capture_opts.h3
-rw-r--r--extcap.c205
-rw-r--r--extcap.h14
-rw-r--r--rawshark.c7
-rw-r--r--tfshark.c2
9 files changed, 177 insertions, 139 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt
index af77d11668..e7e4aabb47 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -2457,7 +2457,6 @@ set_target_properties(copy_data_files PROPERTIES FOLDER "Copy Tasks")
# sources common for wireshark, tshark, rawshark and sharkd
add_library(shark_common OBJECT
cfile.c
- extcap.c
extcap_parser.c
file_packet_provider.c
frame_tvbuff.c
@@ -2476,6 +2475,7 @@ if(BUILD_wireshark AND QT_FOUND)
set(WIRESHARK_SRC
file.c
fileset.c
+ extcap.c
${PLATFORM_UI_SRC}
)
set(wireshark_FILES
@@ -2492,6 +2492,7 @@ if(BUILD_logray AND QT_FOUND)
set(LOGRAY_SRC
file.c
fileset.c
+ extcap.c
${PLATFORM_UI_SRC}
)
set(logray_FILES
@@ -2852,6 +2853,7 @@ if(BUILD_tshark)
$<TARGET_OBJECTS:shark_common>
tshark-tap-register.c
tshark.c
+ extcap.c
${TSHARK_TAP_SRC}
)
diff --git a/capture/capture_session.h b/capture/capture_session.h
index 9343827aa1..69acd26b00 100644
--- a/capture/capture_session.h
+++ b/capture/capture_session.h
@@ -124,6 +124,8 @@ capture_session_init(capture_session *cap_session, capture_file *cf,
new_file_fn new_file, new_packets_fn new_packets,
drops_fn drops, error_fn error,
cfilter_error_fn cfilter_error, closed_fn closed);
+
+void capture_process_finished(capture_session *cap_session);
#else
/* dummy is needed because clang throws the error: empty struct has size 0 in C, size 1 in C++ */
diff --git a/capture/capture_sync.c b/capture/capture_sync.c
index 5b6b21ab01..75b0828d26 100644
--- a/capture/capture_sync.c
+++ b/capture/capture_sync.c
@@ -147,6 +147,56 @@ capture_session_init(capture_session *cap_session, capture_file *cf,
cap_session->closed = closed;
}
+void capture_process_finished(capture_session *cap_session)
+{
+ capture_options *capture_opts = cap_session->capture_opts;
+ interface_options *interface_opts;
+ GString *message;
+ guint i;
+
+ if (cap_session->fork_child != WS_INVALID_PID) {
+ /* Child process still running, session is not closed yet */
+ return;
+ }
+
+ for (i = 0; i < capture_opts->ifaces->len; i++) {
+ interface_opts = &g_array_index(capture_opts->ifaces, interface_options, i);
+ if ((interface_opts->if_type == IF_EXTCAP) &&
+ (interface_opts->extcap_pid != WS_INVALID_PID)) {
+ /* Atleast one extcap process did not finish yet, wait for it */
+ return;
+ }
+ }
+
+ /* All child processes finished */
+ if (capture_opts->extcap_terminate_id > 0) {
+ g_source_remove(capture_opts->extcap_terminate_id);
+ capture_opts->extcap_terminate_id = 0;
+ }
+
+ /* Construct message and close session */
+ message = g_string_new(capture_opts->closed_msg);
+ for (i = 0; i < capture_opts->ifaces->len; i++) {
+ interface_opts = &g_array_index(capture_opts->ifaces, interface_options, i);
+ if (interface_opts->if_type != IF_EXTCAP) {
+ continue;
+ }
+
+ if (interface_opts->extcap_stderr != NULL) {
+ if (message->len > 0) {
+ g_string_append(message, "\n");
+ }
+ g_string_append(message, "Error by extcap pipe: ");
+ g_string_append(message, interface_opts->extcap_stderr);
+ }
+ }
+
+ cap_session->closed(cap_session, message->str);
+ g_string_free(message, TRUE);
+ g_free(capture_opts->closed_msg);
+ capture_opts->closed_msg = NULL;
+}
+
/* Append an arg (realloc) to an argc/argv array */
/* (add a string pointer to a NULL-terminated array of string pointers) */
static char **
@@ -303,8 +353,9 @@ sync_pipe_start(capture_options *capture_opts, GPtrArray *capture_comments,
capture_opts_log(LOG_DOMAIN_CAPTURE, LOG_LEVEL_DEBUG, capture_opts);
cap_session->fork_child = WS_INVALID_PID;
+ cap_session->capture_opts = capture_opts;
- if (!extcap_init_interfaces(capture_opts)) {
+ if (!extcap_init_interfaces(cap_session)) {
report_failure("Unable to init extcaps. (tmp fifo already exists?)");
return FALSE;
}
@@ -717,7 +768,6 @@ sync_pipe_start(capture_options *capture_opts, GPtrArray *capture_comments,
}
cap_session->fork_child_status = 0;
- cap_session->capture_opts = capture_opts;
cap_session->cap_data_info = cap_data;
/* we might wait for a moment till child is ready, so update screen now */
@@ -1763,9 +1813,9 @@ sync_pipe_input_cb(gint source, gpointer user_data)
ws_close(cap_session->signal_pipe_write_fd);
#endif
ws_debug("cleaning extcap pipe");
- extcap_if_cleanup(cap_session->capture_opts, &primary_msg);
- cap_session->closed(cap_session, primary_msg);
- g_free(primary_msg);
+ extcap_if_cleanup(cap_session);
+ cap_session->capture_opts->closed_msg = primary_msg;
+ capture_process_finished(cap_session);
return FALSE;
}
diff --git a/capture_opts.c b/capture_opts.c
index a0ac0bb125..d5e008cfe2 100644
--- a/capture_opts.c
+++ b/capture_opts.c
@@ -63,8 +63,10 @@ capture_opts_init(capture_options *capture_opts)
capture_opts->default_options.extcap = NULL;
capture_opts->default_options.extcap_fifo = NULL;
capture_opts->default_options.extcap_args = NULL;
- capture_opts->default_options.extcap_pipedata = NULL;
capture_opts->default_options.extcap_pid = WS_INVALID_PID;
+ capture_opts->default_options.extcap_pipedata = NULL;
+ capture_opts->default_options.extcap_stderr = NULL;
+ capture_opts->default_options.extcap_child_watch = 0;
#ifdef _WIN32
capture_opts->default_options.extcap_pipe_h = INVALID_HANDLE_VALUE;
capture_opts->default_options.extcap_control_in_h = INVALID_HANDLE_VALUE;
@@ -129,6 +131,8 @@ capture_opts_init(capture_options *capture_opts)
capture_opts->print_name_to = NULL;
capture_opts->temp_dir = NULL;
capture_opts->compress_type = NULL;
+ capture_opts->closed_msg = NULL;
+ capture_opts->extcap_terminate_id = 0;
}
void
@@ -155,6 +159,15 @@ capture_opts_cleanup(capture_options *capture_opts)
}
g_free(capture_opts->save_file);
g_free(capture_opts->temp_dir);
+
+ if (capture_opts->closed_msg) {
+ g_free(capture_opts->closed_msg);
+ capture_opts->closed_msg = NULL;
+ }
+ if (capture_opts->extcap_terminate_id > 0) {
+ g_source_remove(capture_opts->extcap_terminate_id);
+ capture_opts->extcap_terminate_id = 0;
+ }
}
/* log content of capture_opts */
@@ -773,6 +786,8 @@ capture_opts_add_iface_opt(capture_options *capture_opts, const char *optarg_str
interface_opts.extcap_args = NULL;
interface_opts.extcap_pid = WS_INVALID_PID;
interface_opts.extcap_pipedata = NULL;
+ interface_opts.extcap_stderr = NULL;
+ interface_opts.extcap_child_watch = 0;
#ifdef _WIN32
interface_opts.extcap_pipe_h = INVALID_HANDLE_VALUE;
interface_opts.extcap_control_in_h = INVALID_HANDLE_VALUE;
@@ -1274,6 +1289,7 @@ capture_opts_del_iface(capture_options *capture_opts, guint if_index)
if (interface_opts->extcap_pid != WS_INVALID_PID)
ws_pipe_close((ws_pipe_t *) interface_opts->extcap_pipedata);
g_free(interface_opts->extcap_pipedata);
+ g_free(interface_opts->extcap_stderr);
g_free(interface_opts->extcap_control_in);
g_free(interface_opts->extcap_control_out);
#ifdef HAVE_PCAP_REMOTE
@@ -1328,6 +1344,7 @@ collect_ifaces(capture_options *capture_opts)
if (interface_opts.extcap_args)
g_hash_table_ref(interface_opts.extcap_args);
interface_opts.extcap_pipedata = NULL;
+ interface_opts.extcap_stderr = NULL;
#ifdef _WIN32
interface_opts.extcap_pipe_h = INVALID_HANDLE_VALUE;
interface_opts.extcap_control_in_h = INVALID_HANDLE_VALUE;
diff --git a/capture_opts.h b/capture_opts.h
index e1d3ff7960..8a29b8299b 100644
--- a/capture_opts.h
+++ b/capture_opts.h
@@ -210,6 +210,7 @@ typedef struct interface_options_tag {
GHashTable *extcap_args;
GPid extcap_pid; /* pid of running process or WS_INVALID_PID */
gpointer extcap_pipedata;
+ gchar *extcap_stderr;
guint extcap_child_watch;
#ifdef _WIN32
HANDLE extcap_pipe_h;
@@ -326,6 +327,8 @@ typedef struct capture_options_tag {
gboolean output_to_pipe; /**< save_file is a pipe (named or stdout) */
gboolean capture_child; /**< hidden option: Wireshark child mode */
gchar *compress_type; /**< compress type */
+ gchar *closed_msg; /**< Dumpcap capture closed message */
+ guint extcap_terminate_id; /**< extcap process termination source ID */
} capture_options;
/* initialize the capture_options with some reasonable values */
diff --git a/extcap.c b/extcap.c
index 2e2a964958..f7b8c1c10f 100644
--- a/extcap.c
+++ b/extcap.c
@@ -44,6 +44,7 @@
#include <wsutil/wslog.h>
#include <wsutil/ws_assert.h>
+#include "capture/capture_session.h"
#include "capture_opts.h"
#include "extcap.h"
@@ -51,7 +52,10 @@
#include "ui/version_info.h"
-static void extcap_child_watch_cb(GPid pid, gint status, gpointer user_data);
+/* Number of seconds to wait for extcap process to exit after cleanup.
+ * If extcap does not exit before the timeout, it is forcefully terminated.
+ */
+#define EXTCAP_CLEANUP_TIMEOUT 30
/* internal container, for all the extcap executables that have been found.
* Will be reset if extcap_clear_interfaces() is being explicitly called
@@ -1157,14 +1161,13 @@ extcap_has_toolbar(const char *ifname)
return FALSE;
}
-void extcap_if_cleanup(capture_options *capture_opts, gchar **errormsg)
+#ifdef HAVE_LIBPCAP
+static gboolean extcap_terminate_cb(gpointer user_data)
{
+ capture_session *cap_session = (capture_session *)user_data;
+ capture_options *capture_opts = cap_session->capture_opts;
interface_options *interface_opts;
- ws_pipe_t *pipedata;
- guint icnt = 0;
- gboolean overwrite_exitcode;
- gchar *buffer;
-#define STDERR_BUFFER_SIZE 1024
+ guint icnt;
for (icnt = 0; icnt < capture_opts->ifaces->len; icnt++)
{
@@ -1177,7 +1180,37 @@ void extcap_if_cleanup(capture_options *capture_opts, gchar **errormsg)
continue;
}
- overwrite_exitcode = FALSE;
+ if (interface_opts->extcap_pid != WS_INVALID_PID)
+ {
+#ifdef _WIN32
+ /* extcap_if_cleanup() already called TerminateProcess() */
+#else
+ kill(interface_opts->extcap_pid, SIGKILL);
+#endif
+ }
+ }
+
+ capture_opts->extcap_terminate_id = 0;
+ return G_SOURCE_REMOVE;
+}
+
+void extcap_if_cleanup(capture_session *cap_session)
+{
+ capture_options *capture_opts = cap_session->capture_opts;
+ interface_options *interface_opts;
+ guint icnt = 0;
+ gboolean extcaps_alive = FALSE;
+
+ for (icnt = 0; icnt < capture_opts->ifaces->len; icnt++)
+ {
+ interface_opts = &g_array_index(capture_opts->ifaces, interface_options,
+ icnt);
+
+ /* skip native interfaces */
+ if (interface_opts->if_type != IF_EXTCAP)
+ {
+ continue;
+ }
ws_debug("Extcap [%s] - Cleaning up fifo: %s; PID: %d", interface_opts->name,
interface_opts->extcap_fifo, interface_opts->extcap_pid);
@@ -1223,98 +1256,26 @@ void extcap_if_cleanup(capture_options *capture_opts, gchar **errormsg)
ws_unlink(interface_opts->extcap_control_out);
interface_opts->extcap_control_out = NULL;
}
+#endif
/* Send termination signal to child. On Linux and OSX the child will not notice that the
* pipe has been closed before writing to the pipe.
*/
if (interface_opts->extcap_pid != WS_INVALID_PID)
{
+ extcaps_alive = TRUE;
+#ifdef _WIN32
+ /* Not nice, but Wireshark has been doing so for years */
+ TerminateProcess(interface_opts->extcap_pid, 0);
+#else
kill(interface_opts->extcap_pid, SIGTERM);
- }
-#endif
- /* Maybe the client closed and removed fifo, but ws should check if
- * pid should be closed */
- ws_debug("Extcap [%s] - Closing spawned PID: %d", interface_opts->name,
- interface_opts->extcap_pid);
-
- pipedata = (ws_pipe_t *) interface_opts->extcap_pipedata;
- if (pipedata)
- {
- if (pipedata->stderr_fd > 0)
- {
- buffer = (gchar *)g_malloc0(STDERR_BUFFER_SIZE + 1);
- ws_read_string_from_pipe(ws_get_pipe_handle(pipedata->stderr_fd), buffer, STDERR_BUFFER_SIZE + 1);
- if (strlen(buffer) > 0)
- {
- pipedata->stderr_msg = g_strdup(buffer);
- pipedata->exitcode = 1;
- }
- g_free(buffer);
- }
-
-#ifndef _WIN32
- /* Final child watch may not have been called */
- if (interface_opts->extcap_child_watch > 0)
- {
- extcap_child_watch_cb(pipedata->pid, 0, capture_opts);
- /* it will have changed in extcap_child_watch_cb */
- interface_opts = &g_array_index(capture_opts->ifaces, interface_options,
- icnt);
- }
#endif
-
- if (pipedata->stderr_msg != NULL)
- {
- overwrite_exitcode = TRUE;
- }
-
- if (overwrite_exitcode || pipedata->exitcode != 0)
- {
- if (pipedata->stderr_msg != NULL)
- {
- if (*errormsg == NULL)
- {
- *errormsg = ws_strdup_printf("Error by extcap pipe: %s", pipedata->stderr_msg);
- }
- else
- {
- gchar *temp = g_strconcat(*errormsg, "\nError by extcap pipe: " , pipedata->stderr_msg, NULL);
- g_free(*errormsg);
- *errormsg = temp;
- }
- g_free(pipedata->stderr_msg);
- }
-
- pipedata->stderr_msg = NULL;
- pipedata->exitcode = 0;
- }
- }
-
- if (interface_opts->extcap_child_watch > 0)
- {
- g_source_remove(interface_opts->extcap_child_watch);
- interface_opts->extcap_child_watch = 0;
}
+ }
- if (pipedata) {
- if (pipedata->stdout_fd > 0)
- {
- ws_close(pipedata->stdout_fd);
- }
-
- if (pipedata->stderr_fd > 0)
- {
- ws_close(pipedata->stderr_fd);
- }
-
- if (interface_opts->extcap_pid != WS_INVALID_PID)
- {
- ws_pipe_close(pipedata);
- interface_opts->extcap_pid = WS_INVALID_PID;
-
- g_free(pipedata);
- interface_opts->extcap_pipedata = NULL;
- }
- }
+ if (extcaps_alive)
+ {
+ capture_opts->extcap_terminate_id =
+ g_timeout_add_seconds(EXTCAP_CLEANUP_TIMEOUT, extcap_terminate_cb, cap_session);
}
}
@@ -1338,17 +1299,13 @@ extcap_add_arg_and_remove_cb(gpointer key, gpointer value, gpointer data)
return FALSE;
}
-void extcap_child_watch_cb(GPid pid, gint status, gpointer user_data)
+static void extcap_child_watch_cb(GPid pid, gint status _U_, gpointer user_data)
{
guint i;
interface_options *interface_opts;
ws_pipe_t *pipedata = NULL;
- capture_options *capture_opts = (capture_options *)(user_data);
-
- if (capture_opts == NULL || capture_opts->ifaces == NULL || capture_opts->ifaces->len == 0)
- {
- return;
- }
+ capture_session *cap_session = (capture_session *)(user_data);
+ capture_options *capture_opts = cap_session->capture_opts;
/* Close handle to child process. */
g_spawn_close_pid(pid);
@@ -1359,36 +1316,39 @@ void extcap_child_watch_cb(GPid pid, gint status, gpointer user_data)
interface_opts = &g_array_index(capture_opts->ifaces, interface_options, i);
if (interface_opts->extcap_pid == pid)
{
+ ws_debug("Extcap [%s] - Closing spawned PID: %d", interface_opts->name,
+ interface_opts->extcap_pid);
+ interface_opts->extcap_pid = WS_INVALID_PID;
+
pipedata = (ws_pipe_t *)interface_opts->extcap_pipedata;
if (pipedata != NULL)
{
- interface_opts->extcap_pid = WS_INVALID_PID;
- pipedata->exitcode = 0;
-#ifndef _WIN32
- if (WIFEXITED(status))
- {
- if (WEXITSTATUS(status) != 0)
- {
- pipedata->exitcode = WEXITSTATUS(status);
- }
- }
- else
- {
- pipedata->exitcode = G_SPAWN_ERROR_FAILED;
- }
-#else
- if (status != 0)
+ if (pipedata->stdout_fd > 0)
{
- pipedata->exitcode = status;
+ ws_close(pipedata->stdout_fd);
}
-#endif
- if (status == 0 && pipedata->stderr_msg != NULL)
+
+ if (pipedata->stderr_fd > 0)
{
- pipedata->exitcode = 1;
+#define STDERR_BUFFER_SIZE 1024
+ gchar *buffer = (gchar *)g_malloc0(STDERR_BUFFER_SIZE + 1);
+ ws_read_string_from_pipe(ws_get_pipe_handle(pipedata->stderr_fd), buffer, STDERR_BUFFER_SIZE + 1);
+ if (strlen(buffer) > 0)
+ {
+ interface_opts->extcap_stderr = g_strdup(buffer);
+ }
+ g_free(buffer);
+ ws_close(pipedata->stderr_fd);
}
+
+ g_free(pipedata);
+ interface_opts->extcap_pipedata = NULL;
}
- g_source_remove(interface_opts->extcap_child_watch);
+
interface_opts->extcap_child_watch = 0;
+
+ /* Close session if this is the last remaining process */
+ capture_process_finished(cap_session);
break;
}
}
@@ -1584,8 +1544,9 @@ static gboolean extcap_create_pipe(const gchar *ifname, gchar **fifo, const gcha
/* call mkfifo for each extcap,
* returns FALSE if there's an error creating a FIFO */
gboolean
-extcap_init_interfaces(capture_options *capture_opts)
+extcap_init_interfaces(capture_session *cap_session)
{
+ capture_options *capture_opts = cap_session->capture_opts;
guint i;
interface_options *interface_opts;
ws_pipe_t *pipedata;
@@ -1657,7 +1618,8 @@ extcap_init_interfaces(capture_options *capture_opts)
interface_opts->extcap_pid = pid;
interface_opts->extcap_child_watch =
- g_child_watch_add(pid, extcap_child_watch_cb, (gpointer)capture_opts);
+ g_child_watch_add_full(G_PRIORITY_HIGH, pid, extcap_child_watch_cb,
+ (gpointer)cap_session, NULL);
#ifdef _WIN32
/* On Windows, wait for extcap to connect to named pipe.
@@ -1689,6 +1651,7 @@ extcap_init_interfaces(capture_options *capture_opts)
return TRUE;
}
+#endif /* HAVE_LIBPCAP */
/************* EXTCAP LOAD INTERFACE LIST ***************
*
diff --git a/extcap.h b/extcap.h
index 578d2162d1..867c34ffc0 100644
--- a/extcap.h
+++ b/extcap.h
@@ -22,6 +22,7 @@
#include <wsutil/plugins.h>
+#include "capture/capture_session.h"
#include <ui/capture_ui_utils.h>
/* As boolean flags will be allowed any form of yes, true or any number != 0 (or starting with 0)
@@ -205,22 +206,23 @@ extcap_has_configuration(const char * ifname, gboolean is_required);
gboolean
extcap_has_toolbar(const char *ifname);
+#ifdef HAVE_LIBPCAP
/**
- * Initializes each extcap interface with the supplied capture options.
+ * Initializes each extcap interface with the supplied capture session.
* Initializes the extcap interface list if that hasn't already been done.
- * @param capture_opts Capture options.
+ * @param cap_session Capture session.
* @return TRUE on success, FALSE on failure.
*/
gboolean
-extcap_init_interfaces(capture_options * capture_opts);
+extcap_init_interfaces(capture_session *cap_session);
+#endif /* HAVE_LIBPCAP */
/**
* Clean up all if related stuff.
- * @param capture_opts Capture options.
- * @param errormsg Set to NULL on success, error description on failure.
+ * @param cap_session Capture session.
*/
void
-extcap_if_cleanup(capture_options * capture_opts, gchar ** errormsg);
+extcap_if_cleanup(capture_session *cap_session);
/**
* Fetch an extcap preference for a given argument.
diff --git a/rawshark.c b/rawshark.c
index d49c96282d..ceee3998a2 100644
--- a/rawshark.c
+++ b/rawshark.c
@@ -52,6 +52,10 @@
#include <wsutil/wslog.h>
#include <ui/clopts_common.h>
+#ifdef _WIN32
+#include <wsutil/unicode-utils.h>
+#endif
+
#include "globals.h"
#include <epan/packet.h>
#include <epan/ftypes/ftypes.h>
@@ -84,8 +88,6 @@
#include "capture/capture-pcap-util.h"
-#include "extcap.h"
-
#ifdef HAVE_LIBPCAP
#include <setjmp.h>
#ifdef _WIN32
@@ -812,7 +814,6 @@ clean_exit:
g_free(pipe_name);
epan_free(cfile.epan);
epan_cleanup();
- extcap_cleanup();
wtap_cleanup();
return ret;
}
diff --git a/tfshark.c b/tfshark.c
index 5dd1461e9f..c4630ce456 100644
--- a/tfshark.c
+++ b/tfshark.c
@@ -62,7 +62,6 @@
#include <epan/tap.h>
#include <epan/stat_tap_ui.h>
#include <epan/ex-opt.h>
-#include "extcap.h"
#include <wiretap/wtap-int.h>
#include <wiretap/file_wrappers.h>
@@ -966,7 +965,6 @@ clean_exit:
destroy_print_stream(print_stream);
epan_free(cfile.epan);
epan_cleanup();
- extcap_cleanup();
output_fields_free(output_fields);
output_fields = NULL;