Why one should not hide a structure implementation that way?

Question

I've seen some C/C++ code using a trick to hide structure implementation using an opaque (shadow) structure of the same size:

In private.h, the exact implementation of the structure is declared:

typedef struct private_struct
{
    private_foo_t f1;
    private_bar_t b[2];
    private_baz_t *bz;
    int val;
} private_t;

#define PRIVATE_SIZE (sizeof(private_t))

In public.h, the public structure is declared to hold an opaque array of bytes:

#include "private.h"

typedef struct public_struct
{
    char opaque[PRIVATE_SIZE];
} public_t;

public_t and private_t share the same size.

Users can allocate themself storage for private implementation using the public structure:

#include <public.h>

int main(void)
{
    public_t pub;

    return public_api(&pub);
}

Implementation can access the hidden implementation:

#include "private.h"

int public_api(public_t *pub)
{
    private_t *priv = (private_t *) pub;

    return priv->val;
}

This seems a pretty neat trick to allow users to allocate storage for variables (eg. declare static variables).

I'm porting proprietary source code using this trick on various embedded system, but I'm not feeling confident in the way structure pub_t is declared.

What can be wrong with this trick ?

Answer 1

In most cases the nature of a internal structure is hidden from public because you want to be free to change it without having to recompile all the code that is using it. And exactly that is what you loose if you use the trick vou have mentioned and the size of private_t changes. So to be free, it's better to provide either a function like alloc_struct() that allocates a structure and returns a void * or a function that returns sizeof(private_t) so that can be used for allocating…

Answer 2

Here's what is wrong with it in C++. From 3.8 [basic.life]:

The lifetime of an object of type T begins when:

• storage with the proper alignment and size for type T is obtained, and
• if the object has non-trivial initialization, its initialization is complete.

and later

For an object of a class type with a non-trivial destructor, the program is not required to call the destructor explicitly before the storage which the object occupies is reused or released; however, if there is no explicit call to the destructor or if a delete-expression (5.3.5) is not used to release the storage, the destructor shall not be implicitly called and any program that depends on the side effects produced by the destructor has undefined behavior.

Others have already pointed out the potential alignment issues, which also exist in C. But in C++ initialization is a special problem. The public user isn't performing any, so you can only cast the pointer to the private type and use it if the private type has no initialization. There's a parallel problem with destruction -- you force the private object to have trivial destruction.

Which clearly is why you've written private_baz_t *bz; when you should be using a smart pointer.

The only "benefits" this trick buys you are memory leaks and lack of exception safety -- all the things RAII is designed to protect against. Use the p/impl pattern instead, which actually provides a compilation firewall and improves build times.

Answer 3

Beware of Alignment !

public_t native alignment is 1 since char are aligned to 1 byte. private_t alignment is set to the highest alignment requirement of its members, which is certainly not 1. It's probably aligned on the size of a pointer (void *), but there's a double inside a substructure which might require an alignment on 8 bytes. You may seen various kind of alignment depending on the ABI.

Let's try a example program, compiled and tested on i386/i686 with gcc (code source follow):

     kind         name       address   size   alignment   required

     type |      foo_t |         N/A |   48 |       N/A |        4 
     type |     priv_t |         N/A |   56 |       N/A |        4 
     type |      pub_t |         N/A |   56 |       N/A |        1 

   object |       u8_0 |  0xfff72caf |    1 |         1 |        1
   object |       u8_1 |  0xfff72cae |    1 |         2 |        1
   object |       u8_2 |  0xfff72cad |    1 |         1 |        1
   object |       pub0 |  0xfff72c75 |   56 |         1 |        1
   object |       u8_3 |  0xfff72c74 |    1 |         4 |        1
   object |       pub1 |  0xfff72c3c |   56 |         4 |        1
   object |       u8_4 |  0xfff72c3b |    1 |         1 |        1
   object |      priv0 |  0xfff72c00 |   56 |      1024 |        4
   object |       u8_5 |  0xfff72bff |    1 |         1 |        1
   object |      priv1 |  0xfff72bc4 |   56 |         4 |        4
   object |       u8_6 |  0xfff72bc3 |    1 |         1 |        1

  pointer |       pubp |  0xfff72c75 |   56 |         1 |        1
  pointer |      privp |  0xfff72c75 |   56 |         1 |        4  **UNALIGNED**
   object | privp->val |  0xfff72c75 |    4 |         1 |        4  **UNALIGNED**
   object | privp->ptr |  0xfff72c79 |    4 |         1 |        4  **UNALIGNED**
   object |   privp->f |  0xfff72c7d |   48 |         1 |        4  **UNALIGNED**

Source code of the test:

#include <stdalign.h>
#ifdef __cplusplus
/* you will need to pass -std=gnu++11 to g++ */
#include <cstdint>
#endif
#include <stdint.h>
#include <stdio.h>
#include <inttypes.h>

#ifdef __cplusplus
#define alignof __alignof__
#endif

#define PRINTHEADER() printheader()
#define PRINTSPACE() printspace()
#define PRINTALIGN(obj) printobjalign("object", #obj, &obj, sizeof(obj), alignof(obj))
#define PRINTALIGNP(ptr) printobjalign("pointer", #ptr, ptr, sizeof(*ptr), alignof(*ptr))
#define PRINTALIGNT(type) printtypealign(#type, sizeof(type), alignof(type))

static void
printheader(void)
{
    printf(" %8s   %10s   %18s   %4s   %9s   %8s\n", "kind", "name", "address", "size", "alignment", "required");
}

static void
printspace(void)
{
    printf(" %8s   %10s   %18s   %4s   %9s   %8s\n", "", "", "", "", "", "");
}

static void
printtypealign(const char *name, size_t szof, size_t alof)
{
    printf(" %8s | %10s | %18s | %4zu | %9s | %8zu \n", "type", name, "N/A", szof, "N/A", alof);
}

static void
printobjalign(const char *tag, const char *name, const void * ptr, size_t szof, size_t alof)
{
    const uintptr_t uintptr = (uintptr_t)ptr;
    uintptr_t mask = 1;
    size_t align = 0;

    /* get current alignment of the pointer */
    while(mask != UINTPTR_MAX) {

        if ((uintptr & mask) != 0) {
            align = (mask + 1) / 2;
            break;
        }

        mask <<= 1;
        mask |= 1;
    }

    printf(" %8s | %10s | %18p | %4zu | %9zu | %8zu%s\n",
           tag, name, ptr, szof, align, alof, (align < alof) ? "  **UNALIGNED**" : "");
}

/* a foo struct with various fields */
typedef struct foo
{
    uint8_t f8_0;
    uint16_t f16;
    uint8_t f8_1;
    uint32_t f32;
    uint8_t f8_2;
    uint64_t f64;
    uint8_t f8_3;
    double d;
    uint8_t f8_4;
    void *p;
    uint8_t f8_5;
} foo_t;

/* the implementation struct */
typedef struct priv
{
    uint32_t val;
    void *ptr;
    struct foo f;
} priv_t;

/* the opaque struct */
typedef struct pub
{
    uint8_t padding[sizeof(priv_t)];
} pub_t;

static int
test(pub_t *pubp)
{
    priv_t *privp = (priv_t *)pubp;

    PRINTALIGNP(pubp);
    PRINTALIGNP(privp);
    PRINTALIGN(privp->val);
    PRINTALIGN(privp->ptr);
    PRINTALIGN(privp->f);
    PRINTSPACE();

    return privp->val;
}

int
main(void)
{
    uint8_t u8_0;
    uint8_t u8_1;
    uint8_t u8_2;
    pub_t pub0;
    uint8_t u8_3;
    pub_t pub1;
    uint8_t u8_4;
    priv_t priv0;
    uint8_t u8_5;
    priv_t priv1;
    uint8_t u8_6;

    PRINTHEADER();
    PRINTSPACE();

    PRINTALIGNT(foo_t);
    PRINTALIGNT(priv_t);
    PRINTALIGNT(pub_t);
    PRINTSPACE();

    PRINTALIGN(u8_0);
    PRINTALIGN(u8_1);
    PRINTALIGN(u8_2);
    PRINTALIGN(pub0);
    PRINTALIGN(u8_3);
    PRINTALIGN(pub1);
    PRINTALIGN(u8_4);
    PRINTALIGN(priv0);
    PRINTALIGN(u8_5);
    PRINTALIGN(priv1);
    PRINTALIGN(u8_6);
    PRINTSPACE();

    return test(&pub0);
}

Analysis:

pub0 is allocated on the stack and is passed as argument to function test. It is aligned on 1 byte, so that, when cast'ed as a priv_t pointer, priv_t structure members are not aligned.

And that can be bad:

• bad for correctness: some architectures/CPUs will silently corrupt read/write operations to unaligned memory address, some others will generate a fault. The latter is better.
• bad for performance: if supported, unaligned access/load/store are still known to be poorly handled: you're likely asking the CPUs to read/write twice the memory size of the object ... you might hit the cache badly this way.

So, if you really want to hide structure content, you should take care of the alignment of the underlying structure: don't use char.

By default, use void *, or if there can be double in any members of the structure, use double. This will works until someone use a #prama or __attribute__(()) to choose an higher alignment for (a member of) the hidden structure.

Let's define correctly pub_t:

typedef struct pub
{
    double opaque[(sizeof(priv_t) + (sizeof(double) - 1)) / sizeof(double)];
} pub_t;

It might sound complex, and it is ! This way the pub_t structure will have correct alignment and be at least as big as underlying priv_t.

If priv_t was packed (with #pragma or __attribute__(())), using sizeof(priv_t)/sizeof(double), pub_t could be smaller than priv_t ... which will be even worst than the problem we were trying to solve initially. But if the structure was packed, who care of the alignment.

malloc()

If pub_t structure was allocated by malloc() instead of being allocated on stack, the alignment would not be a problem since malloc() is defined to return a memory block aligned to the greatest memory alignments of the C native types, eg. double. In modern malloc() implementations alignment could be up to 32 bytes.