Current usage of intptr_t prevents API mishaps to be detected

[jengelh]

The definition of libpff_item_t is compatible to libpff_record_set_t (and possibly others). This is very unfortunate, because it masks developer errors, for example when a logically different-typed thing is passed:

#include <libpff.h>
void f(libpff_record_set_t *x) {}
int main()
{
        libpff_item_t *x = NULL;
        f(x);
}

Observed:

» gcc x.c -Wall
»

Expected to see:

» gcc x.c -Wall
x.c: In function ‘main’:
x.c:11:4: warning: passing argument 1 of ‘f’ from incompatible pointer type [-Wincompatible-pointer-types]
   11 |  f(x);
x.c:7:29: note: expected ‘libpff_record_set_t *’ but argument is of type ‘libpff_item_t *’

[joachimmetz]

@jengelh what do you propose here as the solution?

I've ruled out

• public structs, these can break at the ABI and resolve in hard to debug issues
• public structs as proxy pointers, these are not supported by every compiler

[jengelh]

Off the top of my head, two options. A forward declaration of an incomplete struct is common:

struct libpff_file;
extern int file_get_item(struct libpff_file *x, int itmidx);

Alternatively, one could wrap intptr into a struct. Since both have the same size, passing the entire struct as an argument shouldn't incur any extra cost.

struct libpff_file {
    uintptr_t realptr; // at this point one could also just use void *.
};
extern int file_get_item(struct libpff_file x, int itmidx);

which is just a more fancy way of the first.

[joachimmetz]

thx for the suggestions I'll take a closer look at these