Re: [PATCH spice-gtk] util: Remove unused GError parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]