[Spice-devel] [PATCH spice-gtk 3/5] util: add unix2dos and dos2unix

[Hans de Goede]

Hi,

On 08/24/2013 04:11 PM, Marc-André Lureau wrote:
> Convert line endings from/to LF/CRLF, in utf8.
> ---
>   gtk/spice-util-priv.h |   2 +
>   gtk/spice-util.c      | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 128 insertions(+)
>
> diff --git a/gtk/spice-util-priv.h b/gtk/spice-util-priv.h
> index ee5a42d..cc559dc 100644
> --- a/gtk/spice-util-priv.h
> +++ b/gtk/spice-util-priv.h
> @@ -29,6 +29,8 @@ gboolean spice_strv_contains(const GStrv strv, const gchar *str);
>   gchar* spice_uuid_to_string(const guint8 uuid[16]);
>   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);
>
>   #if GLIB_CHECK_VERSION(2,32,0)
>   #define STATIC_MUTEX            GMutex
> diff --git a/gtk/spice-util.c b/gtk/spice-util.c
> index 774a145..ebdc6aa 100644
> --- a/gtk/spice-util.c
> +++ b/gtk/spice-util.c
> @@ -19,6 +19,7 @@
>   #ifdef HAVE_CONFIG_H
>   # include "config.h"
>   #endif
> +
>   #include <stdlib.h>
>   #include <string.h>
>   #include <glib-object.h>
> @@ -245,3 +246,128 @@ guint16 spice_make_scancode(guint scancode, gboolean release)
>
>       g_return_val_if_reached(0);
>   }
> +
> +typedef enum {
> +    NEWLINE_TYPE_LF,
> +    NEWLINE_TYPE_CR_LF
> +} NewlineType;
> +
> +static gssize get_line(const gchar *str, gsize len,
> +                       NewlineType type, gsize *nl_len,
> +                       GError **error)
> +{
> +    const gchar *p = str;
> +    gsize nl = 0;
> +
> +    if (type == NEWLINE_TYPE_CR_LF) {
> +        while ((p - str) < len) {
> +            p = g_utf8_strchr(p, len, '\r');
> +            if (!p)
> +                break;
> +            p = g_utf8_next_char(p);
> +            if (g_utf8_get_char(p) == '\n') {
> +                len = (p - str) - 1;
> +                nl = 2;
> +                break;
> +            }
> +        }
> +    } else {
> +        p = g_utf8_strchr(str, len, '\n');
> +        if (p) {
> +            len = p - str;
> +            nl = 1;
> +        }
> +    }

NACK, I still find this utterly unreadable. This can be simplified to:

"""

static gssize get_line(const gchar *str, gsize len,
                       NewlineType type, gsize *nl_len,
                       GError **error)
{
     const gchar *p, *endl;
     gsize nl = 0;

     endl = (type == NEWLINE_TYPE_CR_LF) ? "\r\n" : "\n";
     p = g_strstr_len(str, len, endl);
     if (p) {
         len = p - str;
         nl = strlen(endl);
     }

"""

Which is a lot less lines, much less error prone and much
simpler to parse / understand for humans.

Other then that the patch-set looks good.

Regards,

Hans