2

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 ?

11
  • 2
    On what basis do you say it should not be done this way?
    – doron
    Jul 12, 2013 at 16:03
  • 1
    @doron there's an alignment issue, see the answer : stackoverflow.com/questions/17619015/… Jul 12, 2013 at 16:06
  • 2
    First I though it was like "Attempt #3" of gotw.ca/gotw/028.htm but actually this "trick" seems just stupid to me: this code (besides using reserved keyword for struct names) does not hide anything since public.h directly includes private.h...
    – gx_
    Jul 12, 2013 at 16:13
  • 1
    This really needs two questions, one for C and one for C++. The answers are very different.
    – Ben Voigt
    Jul 12, 2013 at 16:56
  • 1
    @gx_ I've fixed the C++ reserved keywords. Thanks Jul 12, 2013 at 17:02

3 Answers 3

8

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.

12
  • 2
    This covers one of the possible errors you'll see with this approach. I'd have thought that maintainability was a bigger problem, with people adding useful new fields to the private struct and forgetting to add them to the public version.
    – simonc
    Jul 12, 2013 at 15:58
  • I thought that alignment problems like this only come about when you wrap the struct in a struct, or have a non-4 sized struct.
    – Guvante
    Jul 12, 2013 at 16:13
  • 2
    Doesn't sizeof automatically include the padding? See for example ideone.com/fZ7Cuj which appears to be padded to a 4-byte boundary. Jul 12, 2013 at 16:15
  • 1
    @MarkRansom you're quite correct, the compiler add padding at the end of the struct so that an array of the structure got each item aligned. But in the case of the opaque structure, this doesn't help to have correct alignment. Jul 12, 2013 at 16:17
  • @ydroneaud: That padding would be returned by sizeof. You would have to wrap this in another struct to get bad behavior. Wrapping an opaque struct in a struct is probably bad practice in either case.
    – Guvante
    Jul 12, 2013 at 16:19
3

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.

0
3

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…

1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.