[Spice-devel] [PATCH spice-gtk] util: Remove unused GError parameter
Jonathon Jongsma
jjongsma at redhat.com
Thu Sep 1 16:15:24 UTC 2016
On Thu, 2016-09-01 at 16:39 +0200, Pavel Grunt wrote:
> ---
> src/spice-gtk-session.c | 22 ++--------------------
> src/spice-util-priv.h | 4 ++--
> src/spice-util.c | 28 ++++++++--------------------
> tests/util.c | 14 ++++----------
> 4 files changed, 16 insertions(+), 52 deletions(-)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 7b75117..0d0193e 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -736,15 +736,7 @@ static void
> clipboard_got_from_guest(SpiceMainChannel *main, guint selection,
> /* on windows, gtk+ would already convert to LF endings, but
> not on unix */
> if (spice_main_agent_test_capability(s->main,
> VD_AGENT_CAP_GUEST_LINEEND_CRLF)) {
> - GError *err = NULL;
> -
> - conv = spice_dos2unix((gchar*)data, size, &err);
> - if (err) {
> - g_warning("Failed to convert text line ending: %s",
> err->message);
> - g_clear_error(&err);
> - goto end;
> - }
> -
> + conv = spice_dos2unix((gchar*)data, size);
> size = strlen(conv);
> }
>
> @@ -755,7 +747,6 @@ static void
> clipboard_got_from_guest(SpiceMainChannel *main, guint selection,
> 8, data, size);
> }
>
> -end:
> if (g_main_loop_is_running (ri->loop))
> g_main_loop_quit (ri->loop);
>
> @@ -921,17 +912,8 @@ static char
> *fixup_clipboard_text(SpiceGtkSession *self, const char *text, int *
> char *conv = NULL;
> int new_len = *len;
>
> -
> if (spice_main_agent_test_capability(self->priv->main,
> VD_AGENT_CAP_GUEST_LINEEND_CRLF)) {
> - GError *err = NULL;
> -
> - conv = spice_unix2dos(text, *len, &err);
> - if (err) {
> - g_warning("Failed to convert text line ending: %s", err-
> >message);
> - g_clear_error(&err);
> - return NULL;
> - }
> -
> + conv = spice_unix2dos(text, *len);
> new_len = strlen(conv);
> } else {
> /* On Windows, with some versions of gtk+,
> GtkSelectionData::length
> diff --git a/src/spice-util-priv.h b/src/spice-util-priv.h
> index d5e1b8a..38b0deb 100644
> --- a/src/spice-util-priv.h
> +++ b/src/spice-util-priv.h
> @@ -28,8 +28,8 @@ G_BEGIN_DECLS
> gboolean spice_strv_contains(const GStrv strv, const gchar *str);
> const gchar* spice_yes_no(gboolean value);
> guint16 spice_make_scancode(guint scancode, gboolean release);
> -gchar* spice_unix2dos(const gchar *str, gssize len, GError **error);
> -gchar* spice_dos2unix(const gchar *str, gssize len, GError **error);
> +gchar* spice_unix2dos(const gchar *str, gssize len);
> +gchar* spice_dos2unix(const gchar *str, gssize len);
> void spice_mono_edge_highlight(unsigned width, unsigned hight,
> const guint8 *and, const guint8 *xor,
> guint8 *dest);
>
> diff --git a/src/spice-util.c b/src/spice-util.c
> index 81a66fd..86377b6 100644
> --- a/src/spice-util.c
> +++ b/src/spice-util.c
> @@ -284,8 +284,7 @@ typedef enum {
> } NewlineType;
>
> static gssize get_line(const gchar *str, gsize len,
> - NewlineType type, gsize *nl_len,
> - GError **error)
> + NewlineType type, gsize *nl_len)
> {
> const gchar *p, *endl;
> gsize nl = 0;
> @@ -304,19 +303,15 @@ static gssize get_line(const gchar *str, gsize
> len,
>
> static gchar* spice_convert_newlines(const gchar *str, gssize len,
> NewlineType from,
> - NewlineType to,
> - GError **error)
> + NewlineType to)
> {
> - GError *err = NULL;
> gssize length;
> gsize nl;
> GString *output;
> - gboolean free_segment = FALSE;
> gint i;
>
> g_return_val_if_fail(str != NULL, NULL);
> g_return_val_if_fail(len >= -1, NULL);
> - g_return_val_if_fail(error == NULL || *error == NULL, NULL);
> /* only 2 supported combinations */
> g_return_val_if_fail((from == NEWLINE_TYPE_LF &&
> to == NEWLINE_TYPE_CR_LF) ||
> @@ -337,7 +332,7 @@ static gchar* spice_convert_newlines(const gchar
> *str, gssize len,
> output = g_string_sized_new(len * 2 + 1);
>
> for (i = 0; i < len; i += length + nl) {
> - length = get_line(str + i, len - i, from, &nl, &err);
> + length = get_line(str + i, len - i, from, &nl);
> if (length < 0)
> break;
>
> @@ -353,30 +348,23 @@ static gchar* spice_convert_newlines(const
> gchar *str, gssize len,
> }
> }
>
> - if (err) {
> - g_propagate_error(error, err);
> - free_segment = TRUE;
> - }
> -
> - return g_string_free(output, free_segment);
> + return g_string_free(output, FALSE);
> }
>
> G_GNUC_INTERNAL
> -gchar* spice_dos2unix(const gchar *str, gssize len, GError **error)
> +gchar* spice_dos2unix(const gchar *str, gssize len)
> {
> return spice_convert_newlines(str, len,
> NEWLINE_TYPE_CR_LF,
> - NEWLINE_TYPE_LF,
> - error);
> + NEWLINE_TYPE_LF);
> }
>
> G_GNUC_INTERNAL
> -gchar* spice_unix2dos(const gchar *str, gssize len, GError **error)
> +gchar* spice_unix2dos(const gchar *str, gssize len)
> {
> return spice_convert_newlines(str, len,
> NEWLINE_TYPE_LF,
> - NEWLINE_TYPE_CR_LF,
> - error);
> + NEWLINE_TYPE_CR_LF);
> }
>
> static bool buf_is_ones(unsigned size, const guint8 *data)
> diff --git a/tests/util.c b/tests/util.c
> index dcc9770..fd75334 100644
> --- a/tests/util.c
> +++ b/tests/util.c
> @@ -35,7 +35,6 @@ static const struct {
>
> static void test_dos2unix(void)
> {
> - GError *err = NULL;
> gchar *tmp;
> unsigned int i;
>
> @@ -43,22 +42,19 @@ static void test_dos2unix(void)
> if (!(dosunix[i].flags & DOS2UNIX))
> continue;
>
> - tmp = spice_dos2unix(dosunix[i].d, -1, &err);
> + tmp = spice_dos2unix(dosunix[i].d, -1);
> g_assert_cmpstr(tmp, ==, dosunix[i].u);
> - g_assert_no_error(err);
> g_free(tmp);
>
> /* including ending \0 */
> - tmp = spice_dos2unix(dosunix[i].d, strlen(dosunix[i].d) + 1,
> &err);
> + tmp = spice_dos2unix(dosunix[i].d, strlen(dosunix[i].d) +
> 1);
> g_assert_cmpstr(tmp, ==, dosunix[i].u);
> - g_assert_no_error(err);
> g_free(tmp);
> }
> }
>
> static void test_unix2dos(void)
> {
> - GError *err = NULL;
> gchar *tmp;
> unsigned int i;
>
> @@ -66,15 +62,13 @@ static void test_unix2dos(void)
> if (!(dosunix[i].flags & UNIX2DOS))
> continue;
>
> - tmp = spice_unix2dos(dosunix[i].u, -1, &err);
> + tmp = spice_unix2dos(dosunix[i].u, -1);
> g_assert_cmpstr(tmp, ==, dosunix[i].d);
> - g_assert_no_error(err);
> g_free(tmp);
>
> /* including ending \0 */
> - tmp = spice_unix2dos(dosunix[i].u, strlen(dosunix[i].u) + 1,
> &err);
> + tmp = spice_unix2dos(dosunix[i].u, strlen(dosunix[i].u) +
> 1);
> g_assert_cmpstr(tmp, ==, dosunix[i].d);
> - g_assert_no_error(err);
> g_free(tmp);
> }
> }
Presumably Marc-Andre had a situation in mind where there might be a
failure, even if he never implemented the handling for the error. I'm
not sure what situation that would be, though. If He doesn't chime in,
I'm fine with removing the unused error. It does simplify the code. I
would appreciate a little discussion in the commit log about why this
error param was originally added but is not actually needed (if you can
figure out the answers to those questions).
More information about the Spice-devel
mailing list