osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

We're an editor now, as we let you add, delete, and edit frame comments, 

so "Save" should, for non-temporary files, mean "save the current state
of the capture file on top of the existing file" without prompting for a
file name.

That means we have to do a "safe save" - i.e, write the capture out to a
new file and, if that succeeds, rename the new file on top of the old
file - as the actual packet data to write out is in the file we're
overwriting, not in memory.  (We'd want to do that anyway, of
course....)

Update some comments.

Clean up indentation slightly, and get rid of an unnecessary variable
(in all the cases where we use it, we assign it the same value, and that
value isn't modified out from under us before we use it).

Note that after a "Save", or a "Save As" that writes out all captured
packets, we shouldn't have to close the current file and open the new
file and reread it - we should be able to open the new file and update
the frame offsets in the frame_data structures.

Note that we need to do some a better job of reporting rename failures.

svn path=/trunk/; revision=42777
This commit is contained in:
Guy Harris 2012-05-22 10:36:40 +00:00
parent df7289bb99
commit ae7d57d5fa
4 changed files with 179 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

148
file.c
View File

@ -3805,10 +3805,11 @@ cf_can_save_as(capture_file *cf)
return FALSE;
}
cf_status_t
cf_save(capture_file *cf, const char *fname, packet_range_t *range, guint save_format, gboolean compressed)
static cf_status_t
cf_save_packets(capture_file *cf, const char *fname, packet_range_t *range,
guint save_format, gboolean compressed, gboolean do_overwrite)
{
gchar *from_filename;
gchar *fname_new = NULL;
int err;
gboolean do_copy;
wtap_dumper *pdh;
@ -3816,37 +3817,59 @@ cf_save(capture_file *cf, const char *fname, packet_range_t *range, guint save_f
cf_callback_invoke(cf_cb_file_save_started, (gpointer)fname);
/* don't write over an existing file. */
/* this should've been already checked by our caller, just to be sure... */
if (file_exists(fname)) {
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"%sCapture file: \"%s\" already exists!%s\n\n"
"Please choose a different filename.",
simple_dialog_primary_start(), fname, simple_dialog_primary_end());
goto fail;
if (do_overwrite) {
/* We're overwriting an existing file; write out to a new file,
and, if that succeeds, rename the new file on top of the
old file. That makes this a "safe save", so that we don't
lose the old file if we have a problem writing out the new
file. (If the existing file is the current capture file,
we *HAVE* to do that, otherwise we're overwriting the file
from which we're reading the packets that we're writing!) */
fname_new = g_strdup_printf("%s~", fname);
} else {
/* don't write over an existing file. */
/* this should've been already checked by our caller, just to be sure... */
if (file_exists(fname)) {
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"%sCapture file: \"%s\" already exists!%s\n\n"
"Please choose a different filename.",
simple_dialog_primary_start(),
fname,
simple_dialog_primary_end());
goto fail;
}
}
}
packet_range_process_init(range);
if (packet_range_process_all(range) && save_format == cf->cd_t) {
if (packet_range_process_all(range) && save_format == cf->cd_t
&& !cf->unsaved_changes) {
/* We're not filtering packets, and we're saving it in the format
it's already in, so we can just move or copy the raw data. */
it's already in, and there are no changes we have in memory
that aren't saved to the file, so we can just move or copy the
raw data. */
if (cf->is_tempfile) {
/* The file being saved is a temporary file from a live
capture, so it doesn't need to stay around under that name;
first, try renaming the capture buffer file to the new name. */
first, try renaming the capture buffer file to the new name.
XXX - ws_rename() should be ws_stdio_rename() on Windows,
and ws_stdio_rename() uses MoveFileEx() with MOVEFILE_REPLACE_EXISTING,
so it should remove the target if it exists, so this stuff
should be OK even on Windows. */
#ifndef _WIN32
if (ws_rename(cf->filename, fname) == 0) {
/* That succeeded - there's no need to copy the source file. */
from_filename = NULL;
do_copy = FALSE;
do_copy = FALSE;
} else {
if (errno == EXDEV) {
/* They're on different file systems, so we have to copy the
file. */
do_copy = TRUE;
from_filename = cf->filename;
} else {
/* The rename failed, but not because they're on different
file systems - put up an error message. (Or should we
@ -3862,24 +3885,32 @@ cf_save(capture_file *cf, const char *fname, packet_range_t *range, guint save_f
}
#else
do_copy = TRUE;
from_filename = cf->filename;
#endif
} else {
/* It's a permanent file, so we should copy it, and not remove the
original. */
do_copy = TRUE;
from_filename = cf->filename;
}
if (do_copy) {
/* Copy the file, if we haven't moved it. */
if (!copy_file_binary_mode(from_filename, fname))
/* Copy the file, if we haven't moved it.
This does not happen if we have unsaved changes (see above),
so it's either happening as the result of an explicit "Save
As", in which case we've already made sure the target file
doesn't exist, or it's a "Save" on a temporary file for a
capture, in which case the user was asked for the file name
and, again, we've already made sure the target file doesn't
exist. That means we don't need to worry about safe saves. */
if (!copy_file_binary_mode(cf->filename, fname))
goto fail;
}
} else {
/* Either we're filtering packets, or we're saving in a different
format; we can't do that by copying or moving the capture file,
we have to do it by writing the packets out in Wiretap. */
format, or we're saving changes, such as added, modified, or
removed comments, that haven't yet been written to the
underlying file; we can't do that by copying or moving the
capture file, we have to do it by writing the packets out in
Wiretap. */
wtapng_section_t *shb_hdr = NULL;
wtapng_iface_descriptions_t *idb_inf = NULL;
@ -3887,8 +3918,20 @@ cf_save(capture_file *cf, const char *fname, packet_range_t *range, guint save_f
shb_hdr = wtap_file_get_shb_info(cf->wth);
idb_inf = wtap_file_get_idb_info(cf->wth);
pdh = wtap_dump_open_ng(fname, save_format, cf->lnk_t, cf->snap,
compressed, shb_hdr, idb_inf, &err);
if (fname_new != NULL) {
/* We're overwriting an existing file; write out to a new file,
and, if that succeeds, rename the new file on top of the
old file. That makes this a "safe save", so that we don't
lose the old file if we have a problem writing out the new
file. (If the existing file is the current capture file,
we *HAVE* to do that, otherwise we're overwriting the file
from which we're reading the packets that we're writing!) */
pdh = wtap_dump_open_ng(fname_new, save_format, cf->lnk_t, cf->snap,
compressed, shb_hdr, idb_inf, &err);
} else {
pdh = wtap_dump_open_ng(fname, save_format, cf->lnk_t, cf->snap,
compressed, shb_hdr, idb_inf, &err);
}
g_free(idb_inf);
idb_inf = NULL;
@ -3940,6 +3983,27 @@ cf_save(capture_file *cf, const char *fname, packet_range_t *range, guint save_f
}
}
if (fname_new != NULL) {
/* We wrote out to fname_new, and should rename it on top of
fname; fname is now closed, so that should be possible even
on Windows. Do the rename. */
if (ws_rename(fname_new, fname) == -1) {
/* Well, the rename failed. Discard the file we wrote out,
and return an error. */
int rename_err = errno;
/* If this fails, there's nothing we can do to deal with that,
and whatever error it got is less relevant to the user than
the error from the rename failing, so we don't bother
checking for errors in the unlink. */
ws_unlink(fname_new);
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
file_rename_error_message(rename_err), fname);
goto fail;
}
}
cf_callback_invoke(cf_cb_file_save_finished, NULL);
if (packet_range_process_all(range)) {
@ -3952,7 +4016,12 @@ cf_save(capture_file *cf, const char *fname, packet_range_t *range, guint save_f
file be the one we have opened and from which we're reading
the data, and it means we have to spend time opening and
reading the file, which could be a significant amount of
time if the file is large. */
time if the file is large.
If the capture-file-writing code were to return the
seek offset of each packet it writes, we could save that
in the frame_data structure for the frame, and just open
the file without reading it again. */
cf->unsaved_changes = FALSE;
if ((cf_open(cf, fname, FALSE, &err)) == CF_OK) {
@ -3984,6 +4053,23 @@ fail:
return CF_ERROR;
}
cf_status_t
cf_save(capture_file *cf, const char *fname, guint save_format, gboolean compressed)
{
packet_range_t range;
/* This only does a "save all", so we have our own packet_range_t
structure, which is set to the default "save everything" state. */
packet_range_init(&range);
return cf_save_packets(cf, fname, &range, save_format, compressed, TRUE);
}
cf_status_t
cf_save_as(capture_file *cf, const char *fname, packet_range_t *range, guint save_format, gboolean compressed)
{
return cf_save_packets(cf, fname, range, save_format, compressed, FALSE);
}
static void
cf_open_failure_alert_box(const char *filename, int err, gchar *err_info,
gboolean for_writing, int file_type)
@ -4120,6 +4206,11 @@ cf_open_failure_alert_box(const char *filename, int err, gchar *err_info,
}
}
/*
* XXX - whether we mention the source pathname, the target pathname,
* or both depends on the error and on what we find if we look for
* one or both of them.
*/
static const char *
file_rename_error_message(int err)
{
@ -4129,14 +4220,21 @@ file_rename_error_message(int err)
switch (err) {
case ENOENT:
/* XXX - should check whether the source exists and, if not,
report it as the problem and, if so, report the destination
as the problem. */
errmsg = "The path to the file \"%s\" doesn't exist.";
break;
case EACCES:
/* XXX - if we're doing a rename after a safe save, we should
probably say something else. */
errmsg = "You don't have permission to move the capture file to \"%s\".";
break;
default:
/* XXX - this should probably mention both the source and destination
pathnames. */
g_snprintf(errmsg_errno, sizeof(errmsg_errno),
"The file \"%%s\" could not be moved: %s.",
wtap_strerror(err));

17
file.h
View File

@ -196,7 +196,20 @@ cf_read_status_t cf_finish_tail(capture_file *cf, int *err);
gboolean cf_can_save_as(capture_file *cf);
/**
* Save a capture file (or a range of it).
* Save a capture file. Does a "safe save" if the specified
* pathname already exists.
*
* @param cf the capture file to save to
* @param fname the filename to save to
* @param save_format the format of the file to save (libpcap, ...)
* @param compressed whether to gzip compress the file
* @return one of cf_status_t
*/
cf_status_t cf_save(capture_file * cf, const char *fname, guint save_format, gboolean compressed);
/**
* Save a capture file (or a range of it). Fails if the specified
* pathname already exists.
*
* @param cf the capture file to save to
* @param fname the filename to save to
@ -205,7 +218,7 @@ gboolean cf_can_save_as(capture_file *cf);
* @param compressed whether to gzip compress the file
* @return one of cf_status_t
*/
cf_status_t cf_save(capture_file * cf, const char *fname, packet_range_t *range, guint save_format, gboolean compressed);
cf_status_t cf_save_as(capture_file * cf, const char *fname, packet_range_t *range, guint save_format, gboolean compressed);
/**
* Get a displayable name of the capture file.

47
ui/gtk/capture_file_dlg.c
View File

@ -1100,16 +1100,20 @@ file_close_cmd_cb(GtkWidget *widget _U_, gpointer data _U_) {
void
file_save_cmd_cb(GtkWidget *w _U_, gpointer data _U_) {
/* If the file has no unsaved changes, and is not a temporary file,
do nothing. */
if (!cfile.unsaved_changes && !cfile.is_tempfile)
return;
/* XXX TODO - if it's not a temporary file, "save" should just save
on top of the existing file. */
/* Do a "Save As". */
file_save_as_cmd(after_save_no_action, NULL, FALSE);
if (cfile.is_tempfile) {
/* This is a temporary capture file, so saving it means saving
it to a permanent file. */
file_save_as_cmd(after_save_no_action, NULL, FALSE);
} else {
if (cfile.unsaved_changes) {
/* This is not a temporary capture file, but it has unsaved
changes, so saving it means doing a "safe save" on top
of the existing file, in the same format - no UI needed. */
file_save_cmd(after_save_no_action, NULL);
}
/* Otherwise just do nothing (this shouldn't even be enabled if
it's not a temporary file and there are no unsaved changes). */
}
}
/* Attach a list of the valid 'save as' file types to a combo_box by
@ -1175,6 +1179,23 @@ action_after_save_e action_after_save_g;
gpointer action_after_save_data_g;
void
file_save_cmd(action_after_save_e action_after_save, gpointer action_after_save_data)
{
char *fname;
action_after_save_g = action_after_save;
action_after_save_data_g = action_after_save_data;
/* XXX - cfile.filename might get freed out from under us, because
the code path through which cf_save() goes currently closes the
current file and then opens and reloads the saved file, so make
a copy and free it later. */
fname = g_strdup(cfile.filename);
cf_save(&cfile, fname, cfile.cd_t, FALSE);
g_free(fname);
}
void
file_save_as_cmd(action_after_save_e action_after_save, gpointer action_after_save_data, gboolean save_only_displayed)
{
@ -1298,11 +1319,11 @@ file_save_as_cb(GtkWidget *w _U_, gpointer fs) {
/* XXX - if the user requests to save to an already existing filename, */
/* ask in a dialog if that's intended */
/* currently, cf_save() will simply deny it */
/* currently, cf_save_as() will simply deny it */
/* Write out the packets (all, or only the ones from the current
range) to the file with the specified name. */
if (cf_save(&cfile, cf_name, &range, file_type,
if (cf_save_as(&cfile, cf_name, &range, file_type,
gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(compressed_cb))) != CF_OK) {
/* The write failed; don't dismiss the open dialog box,
just leave it around so that the user can, after they
@ -1317,7 +1338,7 @@ file_save_as_cb(GtkWidget *w _U_, gpointer fs) {
}
/* The write succeeded; get rid of the file selection box. */
/* cf_save() might already closed our dialog! */
/* cf_save_as() might already closed our dialog! */
if (file_save_as_w)
window_destroy(GTK_WIDGET (fs));

7
ui/gtk/capture_file_dlg.h
View File

@ -42,6 +42,13 @@ typedef enum {
after_save_exit /**< exit program */
} action_after_save_e;
/** Do a "Save", opening a dialog box if the current file is a temporary file.
*
* @param action_after_save the action to take, when save completed
* @param action_after_save_data data for action_after_save
*/
void file_save_cmd(action_after_save_e action_after_save, gpointer action_after_save_data);
/** Open the "Save As" dialog box.
*
* @param action_after_save the action to take, when save completed