``` {=html}
<style>
body { max-width: 48em !important; }
</style>
```

##### [digital-domain.net](https://digital-domain.net/)

# Some common C mistakes

First of all I'm talking specifically about C, _not_ C++. However some of this
_may_ also apply to C++.

## C only supports pass-by-value

C really only supports pass by value. Pass by reference can be accomplished
by using pointers.

A fairly common thing I see in codebases is something like the following

```C
#include <stdio.h>
#include <stdlib.h>

struct s {
        int i;
};

static void f(struct s *s)
{
        /* Do something with s */

        free(s);
        s = NULL;
}

int main(void)
{
        struct s *s = malloc(sizeof(*s));

        printf("s: %p\n", s);
        f(s);
        printf("s: %p\n", s);

        return 0;
}
```

Which produces the following output

```
$ ./pass-by-value
s: 0x8092a0
s: 0x8092a0
```

In the function _f()_ we pass a pointer to a struct. Something is done with
_s_, then it's free(3)'d.

So far, so good, but then a common thing to do is to _NULL_ the free'd
pointer.

However that will have no effect outside _f()_. The pointer address was passed
to _f()_ on the stack and now has no direct connection to _s_ in _main()_.

If you want to really _NULL_ out the pointer, then you need to pass it in by
_reference_ by passing in a pointer to it. E.g

```C
static void f(struct s **s)
{
        /* Do something with *s */

        free(*s);
        *s = NULL;
}
```

then in _main()_ we pass in _s_ like

```C
        f(&s);
```

which now produces

```console
$ ./pass-by-value
s: 0x8092a0
s: (nil)
```

## Function prototypes with no arguments

It is quite common to see things like

```C
void f()
{
        /* Do something */
}
```

where _f()_ takes no arguments. Writing it like the above has different
meanings in C and C++.

In C++ it means a function that takes no arguments. In C it means the number
(and type) of arguments hasn't been specified (note, this is different to
using '...' to specify a variable number of arguments).

The _correct_ way to do this in C is

```C
void f(void)
{
        /* Do something */
}
```

You can enable -Wstrict-prototypes in both _gcc_ and _clang_ to catch these.
E.g

```console
$ gcc -Wstrict-prototypes -c empty-func-args.c
empty-func-args.c:1:6: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
    1 | void f()
      |      ^
```

```console
$ clang -Wstrict-prototypes -c empty-func-args.c
empty-func-args.c:1:7: warning: this old-style function definition is not preceded by a prototype [-Wstrict-prototypes]
void f()
      ^
1 warning generated.
```

## Casting the return type of malloc et al

OK, so this one might be slightly controversial...

Another common thing I see is code like

```C
        int *vals = (int *)malloc(sizeof(int) * nr_vals);
```

Note the casting of the return from malloc(3). While this may be required in
C++, in C

 - It's superfluous. Why clutter the code with unnecessary casts?

 - It can actually hide bugs. OK, how you might ask...

Well lets say you forgot to

```C
#include <stdlib.h>
```

so you don't have the declaration of malloc(3), now the compiler may assume
that the return type of malloc(3) is an int. Now you may be getting a
truncated memory address returned.

NOTE: With current compilers this is not such a problem anymore and you will
get a clear warning about the missing declaration. But the first point still
stands.

## strsep(3) and invalid free(3)

Lets take the following example

```C
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void)
{
        char *str = strdup("Hello World"), *tkn;

        printf("str [%s@%p]\n", str, str);
        tkn = strsep(&str, " ");
        printf("tkn [%s] str [%s@%p]\n", tkn, str, str);

        free(str);

        return 0;
}
```

When run we get the following

```console
$ ./strsep-invalid-free
str [Hello World@0xf77e2a0]
tkn [Hello] str [World@0xf77e2a6]
free(): invalid pointer
Aborted (core dumped)
```

This is due to *str* being modified, as you can see its address changes. Thus
when it’s passed to free(3) *str* is no longer pointing to the address that
was originally allocated.

Rather you need to take a *copy* of the *str* pointer and pass that to
free(3), e.g.

```C
        char *p;
        ...
        p = str;
        /* Do strsep(3)'s */
        free(p);
```

Similarily if you set *str* to *NULL* in strsep(3) or strtok(3) watch you
don't end up just doing *free(NULL)*.