[Spice-devel] [PATCH spice-gtk] util: Remove unused GError parameter
Marc-André Lureau
mlureau at redhat.com
Thu Sep 1 16:26:28 UTC 2016
Hi
----- Original Message -----
> 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).
I don't remember either. I suppose the initial implementation used some function that returned a GError too, but the current implementation doesn't.
So ACK from me
More information about the Spice-devel
mailing list