GFile: Clean up file_copy_fallback to fix SEGV with btrfs

Ok, this function was just an awful mess before.  Now the problem
domain is not trivial, and I won't claim this new code is *beautiful*,
but it should fix the bug at hand, and be somewhat less prone to
failure for the next person who tries to modify it.  There's only one
unref call for each object now.

https://bugzilla.gnome.org/show_bug.cgi?id=692408
This commit is contained in:
Colin Walters 2013-01-25 10:40:45 -05:00
parent 1818e0a2f8
commit 166766a89f

View File

@ -2982,11 +2982,11 @@ file_copy_fallback (GFile *source,
gpointer progress_callback_data, gpointer progress_callback_data,
GError **error) GError **error)
{ {
GInputStream *in; gboolean ret = FALSE;
GOutputStream *out; GInputStream *in = NULL;
GFileInfo *info; GOutputStream *out = NULL;
GFileInfo *info = NULL;
const char *target; const char *target;
gboolean result;
/* need to know the file type */ /* need to know the file type */
info = g_file_query_info (source, info = g_file_query_info (source,
@ -2994,9 +2994,8 @@ file_copy_fallback (GFile *source,
G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
cancellable, cancellable,
error); error);
if (!info)
if (info == NULL) goto out;
return FALSE;
/* Maybe copy the symlink? */ /* Maybe copy the symlink? */
if ((flags & G_FILE_COPY_NOFOLLOW_SYMLINKS) && if ((flags & G_FILE_COPY_NOFOLLOW_SYMLINKS) &&
@ -3006,16 +3005,12 @@ file_copy_fallback (GFile *source,
if (target) if (target)
{ {
if (!copy_symlink (destination, flags, cancellable, target, error)) if (!copy_symlink (destination, flags, cancellable, target, error))
{ goto out;
g_object_unref (info);
return FALSE;
}
g_object_unref (info); ret = TRUE;
goto copied_file; goto out;
} }
/* ... else fall back on a regular file copy */ /* ... else fall back on a regular file copy */
g_object_unref (info);
} }
/* Handle "special" files (pipes, device nodes, ...)? */ /* Handle "special" files (pipes, device nodes, ...)? */
else if (g_file_info_get_file_type (info) == G_FILE_TYPE_SPECIAL) else if (g_file_info_get_file_type (info) == G_FILE_TYPE_SPECIAL)
@ -3023,16 +3018,14 @@ file_copy_fallback (GFile *source,
/* FIXME: could try to recreate device nodes and others? */ /* FIXME: could try to recreate device nodes and others? */
g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
_("Can't copy special file")); _("Can't copy special file"));
g_object_unref (info); goto out;
return FALSE;
} }
/* Everything else should just fall back on a regular copy. */ /* Everything else should just fall back on a regular copy. */
else
g_object_unref (info);
in = open_source_for_copy (source, destination, flags, cancellable, error); in = open_source_for_copy (source, destination, flags, cancellable, error);
if (in == NULL) if (!in)
return FALSE; goto out;
if (flags & G_FILE_COPY_OVERWRITE) if (flags & G_FILE_COPY_OVERWRITE)
{ {
@ -3047,29 +3040,33 @@ file_copy_fallback (GFile *source,
out = (GOutputStream *)g_file_create (destination, 0, cancellable, error); out = (GOutputStream *)g_file_create (destination, 0, cancellable, error);
} }
if (out == NULL) if (!out)
{ goto out;
g_object_unref (in);
return FALSE;
}
#ifdef HAVE_SYS_IOCTL_H #ifdef HAVE_SYS_IOCTL_H
if (G_IS_FILE_DESCRIPTOR_BASED (in) && G_IS_FILE_DESCRIPTOR_BASED (out)) if (G_IS_FILE_DESCRIPTOR_BASED (in) && G_IS_FILE_DESCRIPTOR_BASED (out))
{ {
GError *reflink_err = NULL; GError *reflink_err = NULL;
result = btrfs_reflink_with_progress (in, out, info, cancellable, if (!btrfs_reflink_with_progress (in, out, info, cancellable,
progress_callback, progress_callback_data, progress_callback, progress_callback_data,
&reflink_err); &reflink_err))
if (result || !g_error_matches (reflink_err, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED))
{ {
if (!result) if (g_error_matches (reflink_err, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED))
g_propagate_error (error, reflink_err); {
goto cleanup; g_clear_error (&reflink_err);
} }
else else
g_clear_error (&reflink_err); {
g_propagate_error (error, reflink_err);
goto out;
}
}
else
{
ret = TRUE;
goto out;
}
} }
#endif #endif
@ -3078,45 +3075,60 @@ file_copy_fallback (GFile *source,
{ {
GError *splice_err = NULL; GError *splice_err = NULL;
result = splice_stream_with_progress (in, out, cancellable, if (!splice_stream_with_progress (in, out, cancellable,
progress_callback, progress_callback_data, progress_callback, progress_callback_data,
&splice_err); &splice_err))
if (result || !g_error_matches (splice_err, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED))
{ {
if (!result) if (g_error_matches (splice_err, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED))
g_propagate_error (error, splice_err); {
goto cleanup; g_clear_error (&splice_err);
} }
else else
g_clear_error (&splice_err); {
g_propagate_error (error, splice_err);
goto out;
}
}
else
{
ret = TRUE;
goto out;
}
} }
#endif #endif
result = copy_stream_with_progress (in, out, source, cancellable,
/* A plain read/write loop */
if (!copy_stream_with_progress (in, out, source, cancellable,
progress_callback, progress_callback_data, progress_callback, progress_callback_data,
error); error))
goto out;
cleanup: ret = TRUE;
/* Don't care about errors in source here */ out:
g_input_stream_close (in, cancellable, NULL);
/* But write errors on close are bad! */
if (!g_output_stream_close (out, cancellable, result ? error : NULL))
result = FALSE;
g_object_unref (in);
g_object_unref (out);
if (result == FALSE)
return FALSE;
copied_file:
/* Ignore errors here. Failure to copy metadata is not a hard error */ /* Ignore errors here. Failure to copy metadata is not a hard error */
g_file_copy_attributes (source, destination, if (ret)
(void) g_file_copy_attributes (source, destination,
flags, cancellable, NULL); flags, cancellable, NULL);
return TRUE; if (in)
{
/* Don't care about errors in source here */
(void) g_input_stream_close (in, cancellable, NULL);
g_object_unref (in);
}
if (out)
{
/* But write errors on close are bad! */
if (!g_output_stream_close (out, cancellable, ret ? error : NULL))
ret = FALSE;
g_object_unref (out);
}
g_clear_object (&info);
return ret;
} }
/** /**