Bugs in C

This is my list of my most-hated misfeatures of C

Pass-by-reference arrays

It is not possible for an array type to appear in the parameter list of a function. If you attempt to put an array type in the parameter list of a function, its type is implicitly converted to a pointer to the element type of the array.

Most operations on arrays in C result in the array being implicitly treated as a pointer to its first argument so this usually goes unnoticed. There are two major exceptions that have caused me problems in the past.

sizeof operator

The sizeof operator as applied to an array returns the total storage size of an array. This is useful if you want to copy the array using memcpy() or zero it out using memset(), etc.

void
function (void)
{
  guint array1[5];
  guint array2[5];

  ...

  memcpy (array2, array1, sizeof array2);
  memset (array1, 0, sizeof array1);
}

The above code is easy to read and makes sense.

Consider the following:

void
copy_and_clear (guint dest[5],
                guint src[5])
{
  memcpy (dest, src, sizeof dest);
  memset (src, 0, sizeof src);
}

Depending on if you are on a 32bit or 64bit machine, this will either copy only the first element or only the first two elements.

As stated above, array types on parameter lists are implicitly converted to their equivalent pointer type, so this code is equivalent to:

void
copy_and_clear (guint *dest,
                guint *src)
{
  memcpy (dest, src, sizeof dest);   /* note: sizeof (guint*) */
  memset (src, 0, sizeof src);       /* note: sizeof (guint*) */
}

At which point the problem is clear.

reference operator

The other problem is that the reference operator applied to an array is not equivalent to the reference operator applied to its pointer type.

Consider this example:

static gint
collect_one_int (va_list *app)
{
  return va_arg (*app, gint);
}

gint *
collect_intsv (int n_ints,
               va_list ap)
{
  gint *result;
  gint i;

  result = g_new (gint, n_ints);
  for (i = 0; i < n_ints; i++)
    result[i] = collect_one_int (&ap);

  return result;
}

gint *
collect_ints (int n_ints,
              ...)
{
  gint *result;
  va_list ap;

  va_start (ap, n_ints);
  result = collect_intsv (n_ints, ap);
  va_end (ap);

  return result;
}

C99 guarantees that passing a va_list by reference is well-defined, but the above code is still invalid.

The reason is that va_list can be (and often is) defined as a typedef of an array type. For example, on amd64, it is defined (indirectly) thus:

typedef struct __va_list_tag va_list[1];

That means that a va_list defined locally has an array type, but a va_list appearing on the parameter list of a function has the 'equivalent' pointer type. Taking the address of the local gives a pointer of type struct __va_list_tag (*)[1] but taking the address of the parameter gives a pointer of type struct __va_list_tag **. What's worse is that the actual pointer values are not even the same. The pointer to the array points directly to the array, whereas the pointer to parameter is a pointer to a pointer to the array.

gcc will correctly complain about the above code with a confusing message like:

In function ‘collect_intsv’:
warning: passing argument 1 of ‘collect_one_int’ from incompatible pointer type
     result[i] = collect_one_int (&ap);
                                  ^
note: expected ‘struct __va_list_tag (*)[1]’ but argument is of type ‘struct __va_list_tag **’
 collect_one_int (va_list *app)
 ^

Unintelligent implicit casting of qualified pointer types

aka "the const strv problem"

GLib contains several functions that operate on a NULL-terminated array of strings, or "strv" as we call those for short.

It is possible to create arrays of strings in which each string is constant:

const gchar *my_strv[] = { "one", "two", "three", NULL };

and it's also possible to create arrays of strings in which each element is dynamically allocated and may be modified:

gchar **my_strv = g_strsplit ("one,two,three", ",", -1);

Many common operations can be performed on these two (such as measuring the length), but the types are completely incompatible, leading to (gchar **) and (const gchar **) casts everywhere.

What's worse is that C++ actually gets it right.

Why this is a problem

The types gchar ** and const gchar ** are not compatible, and they shouldn't be compatible.

The reason for that can be illustrated by this code:

static void
write_const_str (const gchar **ptr)
{
  *ptr = "your string";
}

static void
crashing_is_nice (void)
{
  gchar *my_string;

  write_const_str (&my_string);
  my_string[0] = 'a';
  g_free (my_string);
}

The above program will produce a warning on the call to write_const_str() for good reason: the pointer is not a pointer to a const gchar * -- it's a pointer to a gchar *. Storing a const gchar * into that pointer would be a mistake because we'd now be able to modify that const value.

The problem is that write_const_str() can be said to be contravariant with respect to the value that was passed by reference. That's because it writes to this value (effectively making 'ptr' an out parameter). The compiler actually doesn't know what the function will do with the value (since it could also read it) so it has no choice but to rule the function to be invariant with respect to its pass-by-reference argument.

We're talking about a single pass-by-reference value here, but the same argument can be equally applied to arrays (which are really just multiple pass-by-reference values in a row).

You can read about Covariance and contravariance on Wikipedia if this sounds interesting.

Why this shouldn't be a problem

In fact, the type of functions that we want to operate on both types of strv are covariant -- that is, they only ever read values.

What's worse is that the compiler actually knows this.

If we use the type const gchar * const * then we tell it that we want to operate on an array (or passed-by-reference value) of type const gchar * and we promise not to write anything.

With this information, the compiler could effectively apply the normal type promotion rules (eg: gchar *const gchar *) to the array element type. The compiler already knows that a pointer to a given type can be implicitly treated as a const pointer to the same type (eg: gchar **gchar * const * or const gchar **const gchar * const *). Putting those two together we could then convert gchar **gchar * const *const gchar * const *, taking the first step by the existing rules and the second step by reasoning through covariance.

Why this is still a problem

C++ elected to allow this sort of covariance in the type of the pointer.

Unfortunately, the C standard has very simple rules when it comes to casting between pointers: qualifiers (like const) can be added to the pointer itself, but the target type of the pointer must be exactly equal.

This was done in the name of simplicity.

0 is NULL

In C, one often expects that NULL might be defined something like follows:

#define NULL ((void *) 0)

but in fact, any numeric expression that evaluates to zero can be treated as a NULL pointer.

That means that although this code is invalid:

  gpointer x = 1;

this code is perfectly fine:

  gpointer x = 0;

What's worse is that it's not just the literal 0. It's any numeric expression that the compiler can evaluate to zero.

  gpointer x = 5 - 5;

is perfectly valid. As is

  gpointer x = sizeof (int) - sizeof (int);

and also

  gpointer x = FALSE;

which leads to the fact that it's perfectly valid to return FALSE; from a function with a pointer type as a return value (but not TRUE).

AllisonRyanLortie/BugsInC (last edited 2016-01-18 17:26:10 by AndreaVeri)