Coding style

Marius Vollmer marius.vollmer at redhat.com
Fri Nov 8 08:37:46 UTC 2013


Stephen Gallagher <sgallagh at redhat.com> writes:

>> - No trailing whitespace, no tab characters - Maximum line length
>>   is 120 characters.
>
> Could we make this 80 characters?

Hmm, no. :-)

At least not now.  The code mostly respects the 120 limit right now, and
this used to be documented in the old HACKING file.  So changing this
would cause quite some work.

>> - Gtk+ coding standards.
>> - C99 is allowed and the default.
>> - __attribute((cleanup)) is allowed, via libgsystem.
>
> I realize I'm likely to get outvoted on this, but I dislike any
> mechanism that encourages lazy memory management. It's twelve o'clock;
> do you know where your pointers are?

The jury is still out on this, I'd say.  I like uncluttered code that
shows the logic, and I think "gs_free" and friends can help a lot.  I
think a function like this is quite readable and maintainable:

    gchar *
    func ()
    {
      gs_free gchar *last_name = dup_last_name ();
      if (last_name == NULL)
        return NULL;

      gs_free gchar *first_name = dup_first_name (last_name);
      if (first_name == NULL)
        return NULL;

      return g_strdup_printf ("%s %s", first_name, last_name);
    }

This looks much worse to me:

    gchar *
    func ()
    {
      gchar *last_name = NULL;
      gchar *first_name = NULL;
      gchar *result;

      last_name = dup_last_name ();
      if (last_name == NULL)
        return NULL;

      first_name = dup_first_name (last_name);
      if (first_name == NULL)
        {
          g_free (last_name);
          return NULL;
        }

      result = g_strdup_printf ("%s %s", first_name, last_name);
      g_free (first_name);
      g_free (last_name);

      return result;
    }

... mostly because one needs to duplicate the cleanup code a lot.  "goto
out"-style is better:

    gchar *
    func ()
    {
      gchar *last_name = NULL;
      gchar *first_name = NULL;
      gchar *result = NULL;

      last_name = dup_last_name ();
      if (last_name == NULL)
        goto out;

      first_name = dup_first_name (last_name);
      if (first_name == NULL)
        goto out;

      result = g_strdup_printf ("%s %s", first_name, last_name);

    out:
      g_free (first_name);
      g_free (last_name);
      return result;
    }

But using "gs_free" is still better IMO because the return value is much
more explicit.  In a sense, with "gs_free" the compiler is doing the
transformation to "goto out"-style for us, which ought to be a good
thing.


More information about the cockpit-devel mailing list