9

In my opinion, hiding the definition of a structure in C generally makes code safer, as you enforce—with the help of the compiler—that no member of the structure can be accessed directly.

However, it has a downside in that a user of the structure cannot declare variables of its type to be put on the stack, because the size of the structure becomes unavailable this way (and, therefore, the user has to resort to allocating on the heap via malloc() even when it is undesirable).

This can be (partially) solved via the alloca(3) function that is present in all major libc implementations, even though it does not conform to POSIX.

Keeping these pros and cons in mind, can such design be considered good in general?

In lib.h:

struct foo;
extern size_t foo_size;
int foo_get_bar(struct foo *);

In lib.c:

struct foo {
  int bar;
};

size_t foo_size = sizeof foo;

int foo_get_bar(struct foo *foo)
{
  return foo->bar;
}

In example.c:

#include "lib.h"

int bar(void)
{
  struct foo *foo = alloca(foo_size);
  foo_init(foo);
  return foo_get_bar(foo);
}
ib.
  • 27,830
  • 11
  • 80
  • 100
Alexander Solovets
  • 2,447
  • 15
  • 22
  • 6
    The more common choice is to use something like `foo_create` and `foo_destroy`, which means you don't expose *any* details of your structure, and can do more advanced things like storing internally `malloc`'d pointers. There's precious few situations where you *actually* want to use `alloca`, other than perhaps embedded systems where `malloc` and friends are super limited. – Richard J. Ross III Dec 15 '15 at 22:21
  • 2
    If the struct is opaque it would be bad design to have client code needing to allocate or declare any variables of that type as shown in the example. All instances of the struct should come from the library itself. – kaylum Dec 15 '15 at 22:28
  • 3
    `VLA[]` allowed? (C99)? Declaring a character array of `foo_size` (using `alignas`) may work. Yet , in gerneral, agree with @kaylum – chux - Reinstate Monica Dec 15 '15 at 22:31
  • @chqrlie, yes I saw that soon after I posted my comment, then deleted the comment before your response arrived here. – John Bollinger Dec 15 '15 at 22:38
  • Is the desire here to allocate from the stack, or to *not* allocate from the heap? IOW, would it be acceptable to create a large static array of your `struct` type, and have your `foo_create` function simply return a pointer to an element in that array (and do some internal bookkeeping)? You're basically creating and managing your own "heap" without the need of `malloc` or `free`. – John Bode Dec 15 '15 at 22:58
  • 2
    Once you get a candidate solution that meets you goals, clean it up, simplify it and try posting on http://codereview.stackexchange.com for additional feedback. Be prepared for some strong feedback. – chux - Reinstate Monica Dec 15 '15 at 23:19
  • Another thought: What really is the goal of using `alloca()` (or `char cptr[size]`)? If it is to have automatic free'ing on leaving the function/block, I think that is the wrong goal. There are 4 steps to consider: allocating, initialization, closing and de-alllocation. Binding the first pair and binding the last pair as functions makes sense even if the first pair calls `aollca()`. IOWs there are many things to consider -unfortunately a forum is needed and SE is not a forum. – chux - Reinstate Monica Dec 16 '15 at 01:35
  • No, I'd rather use `__attribute__((cleanup))` for automatic deallocation. Here the intention is to not to use `malloc()`. – Alexander Solovets Dec 16 '15 at 02:14

2 Answers2

3

Yes, it is a good practice to hide data.

As an alternate to the alloca(foo_size); pattern, one can declare an aligned character array and perform a pointer conversion. The pointer conversion is not fully portable, though. The character array needs to be a VLA, if the size is defined by a variable and not a compile-time constant:

extern size_t size;

struct sfoo;

#include <stddef.h>

int main(void) {
  unsigned char _Alignas (max_align_t) cptr[size];
  // or unsigned char _Alignas (_Complex  long double) cptr[size];  // some widest type
  struct sfoo *sfooptr = (struct sfoo *) cptr;
  ...

If VLAs are not desired or available, declare the size as a constant (#define foo_N 100) that is guaranteed to be at least as much as needed.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • This idea is interesting, but hardly qualifies as good practice. – chqrlie Dec 15 '15 at 22:58
  • @chqrlie Should you detail the not good practice concerns, perhaps we can address them. "hardly qualifies as good practice." is akin to an OP saying "code does not work". – chux - Reinstate Monica Dec 15 '15 at 23:02
  • 1
    what is *good practice* is probably opinion based. If the price to pay for hiding the implementation is this hard to read VLA contraption, I personally think it is not worth the trouble. What you are proposing is a hack to allow stack allocation, are you going to hide it with a macro? Could that be considered *good practice*? – chqrlie Dec 15 '15 at 23:11
2

Function bar invokes undefined behavior: the structure pointed to by foo is uninitialized.

If you are going to hide the structure details, provide a foo_create() that allocates one and initializes it and foo_finalize that releases any resources and frees it.

What you are proposing could be made to work, but is error prone and is not a general solution.

ib.
  • 27,830
  • 11
  • 80
  • 100
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Fixed the code. As I stated if `foo_create()` is used, then it's impossible to allocate memory on stack which is sometimes desirable. – Alexander Solovets Dec 15 '15 at 22:39
  • @AlexanderSolovets In practice, this is usually undesirable. Allowing objects to be allocated and torn down on the stack isn't great; it means you can't do any cleanup on the instance. –  Dec 15 '15 at 22:40
  • If you want to keep the use of `alloca` you can have a `foo_init` function to initialize it. As chux mentioned in the comments you'd also need the alignment of the struct, so it's just a lot easier for the user to call `foo_create` and not worry about any of that. – Kevin Dec 15 '15 at 22:41
  • @AlexanderSolovets, you asked "Can such a design generally be considered good?" The answer is "no". If you had instead asked "Is it ever a good idea to implement the following pattern?" then perhaps the answer would be different. It would also, however, be much more a matter of opinion, and as such, probably not appropriate for SO. – John Bollinger Dec 15 '15 at 22:41
  • @chqrlie Except the possibility to overflow stack what are the possible errors? – Alexander Solovets Dec 15 '15 at 22:53
  • 1
    @AlexanderSolovets: A possible error is to return `foo` to the caller. Since it looks like a pointer, it is easy to confuse with a pointer returned by `malloc`... I'm not saying you would necessarily make this mistake, but I wouldn't condone this practice because less savvy programmers will be bitten too easily. It is a hack, and may solve a specific problem, but I cannot call this *good practice*. – chqrlie Dec 15 '15 at 23:16