2

I'm hiding some struct fields from struct types to make public API headers more clear.

First I used this style to hide (not actually hide, just separate from public editable members) some members,

include/scene.h

typedef struct ScenePrivateFields {
  SceneFlags flags;
  /* other members */
} ScenePrivateFields;

typedef struct Scene {
  ScenePrivateFields _priv;
  RootNode           rootNode;
  /* other members */
} Scene;

with this style I always use Scene type in function parameters then accessing private fields using _priv member. But keeping all private fields in public header makes header more complex

Eventually I switched to another style

include/scene.h

typedef struct Scene {
  RootNode rootNode;
  /* other members */
} Scene;

EXPORT
Scene*
allocScene(void);

src/types/impl_scene.h

typedef struct SceneImpl {
  Scene      pub;
  SceneFlags flags;
  /* other members */
} SceneImpl;

for instance if I have a function like this:

include/scene.h

void
renderScene(Scene * __restrict scene, /* other params */);

I have to cast scene to SceneImpl to access private fields. I'm doing this like:

src/scene/scene.c

void
renderScene(Scene * __restrict scene, /* other params */) {
  SceneImpl *sceneImpl;

  sceneImpl = (SceneImpl *)scene;
}

to avoid casting every function call I thought that maybe I can do something like this, if it is legal and not violating C standards:

src/scene/scene.c

void
renderScene(SceneImpl * __restrict sceneImpl, /* other params */) {
  /* skip casting scene to sceneImpl */
}

Since Scene is first member of SceneImpl, Can I define public api (function) with Scene and define implementation (function) with SceneImpl. I think it would work because both are pointer, is it valid or good idea?

NOTE: I'm compiling with -fstrict-aliasing

EDIT: FWIW, Here alloc function implementation, users must use this func to alloc struct:

EXPORT
Scene*
allocScene(void) {
  SceneImpl *sceneImpl;

  sceneImpl = calloc(1, sizeof(*sceneImpl));

  /* initialize pulic part */
  sceneImpl->pub.field1 = ...

    /* initialize private part */
  sceneImpl->priv_field1 = ...

  /* return public part */
  return &sceneImpl->pub;
}
recp
  • 383
  • 1
  • 2
  • 14

2 Answers2

1

You can use an opaque type for the private potion of the data.

In your public header, define your structures as follows:

// forward declaration of struct ScenePrivateFields 
typedef struct ScenePrivateFields ScenePrivateFields;

typedef struct Scene {
  ScenePrivateFields *_priv;    // now a pointer
  RootNode           rootNode;
  /* other members */
} Scene;

Then in your implementation file, you define the private struct:

struct ScenePrivateFields {
  SceneFlags flags;
  /* other members */
}

You'll also need to define a function that dynamically creates an instance of struct ScenePrivateFields and return a pointer to it to populate the pointer in the public struct.

dbush
  • 205,898
  • 23
  • 218
  • 273
  • I don't want to extra alloc for private fields, because it will fetch another memory region, I think continuous memory would be better because of cache locality – recp Nov 27 '17 at 19:47
  • @recp You shouldn't worry about that level of optimization unless you have a very specific reason to. Better to stick with well-defined behavior instead of hacks that may not work. – dbush Nov 27 '17 at 19:50
  • I think my struct types are not hack, are they? but I'm thinking on hacking function calls :) this is actual question func calls would be hack or not :/ – recp Nov 27 '17 at 19:54
  • @recp Casting one struct type to another, even if one contains the other, is a hack. You'll need a function to populate the private fields anyway, so better to do so in a well defined manner that actually enforces the field privacy. – dbush Nov 27 '17 at 19:59
  • maybe you're right, because I'm also using strict aliasing. But according to this https://stackoverflow.com/questions/8416417/nested-structs-and-strict-aliasing-in-c, my structures seems well-defined, because A is first member of B, not randomly located inside it, but I'm not sure about the function call, if it is legal and not hack, it would save me from too many base type to derived castings if it is legal – recp Nov 27 '17 at 20:14
  • *I don't want to extra alloc for private fields, because it will fetch another memory region, I think continuous memory would be better because of cache locality* You can't have both private fields and continuous memory with non-private fields. To make them continuous, you have to expose what's inside the structure you want to be private. The compiler has to know what's inside `struct ScenePrivateFields` in order to put that structure inside a `struct Scene`. – Andrew Henle Nov 27 '17 at 20:56
  • @AndrewHenle check SceneImpl definition all members are visible to compiler and they are allocated with public members with single calloc/malloc call. Private members are not exposed but custom alloc function are provided, it allocates SceneImpl and returns &sceneImpl->pub; So users can only use this structure as pointer, they cant alloc it statically. Also all ScenePrivateFields members were public/exposed, but I'm using SceneImpl (which is completely private) now. – recp Nov 27 '17 at 21:13
  • @recp Won't work. You can't access any fields of a `struct` without knowing where every field is. You can not access any part of `typedef struct Scene { ScenePrivateFields _priv;...` without knowing what's in `ScenePrivateFields`. You can not embed an opaque structure inside a non-opaque one. All you can do is provide a pointer in the public structure to an opaque one. If a structure is opaque, you can't even tell how many bytes an instance of it takes up, so how can you embed it into a publicly-visible structure? – Andrew Henle Nov 27 '17 at 21:32
  • @AndrewHenle I'm accessing private fields in my src/ folder and users should not try to access them. I edited my question and provided alloc func. All members are visible while alloc-ing and while using private fields privately :) private fields are library-level. If I want to use private fields then I'm importing extra src/types/scene_impl.h header. So all members are visible with extended struct (Scene + SceneImpl) in library – recp Nov 27 '17 at 21:43
  • @recp The allocation does not matter. Given `typedef struct Scene { ScenePrivateFields _priv; RootNode rootNode;...`, you can not access fields in `struct Scene` without knowing what fields are in `struct ScenePrivateFields`. Go ahead and try writing code that does that. You **can not** write C code that can access `rootNode` unless it also has access to the content of `struct ScenePrivateFields`. – Andrew Henle Nov 27 '17 at 21:49
  • 1
    @AndrewHenle I don't think he is at this point. Right now he's dynamically creating a private struct whose first member is a public struct and returning a pointer to the public member. The rest of the library functions are then casting the public pointer to a private pointer. – dbush Nov 27 '17 at 21:54
  • @AndrewHenle Based on the linked question further up the comment chain it should be allowable, but it feels like a hack. – dbush Nov 27 '17 at 21:57
  • @AndrewHenle here real code: [include/scene.h](https://github.com/recp/libgk/blob/master/include/gk/scene.h) , [src/scene/scene.c (alloc)](https://github.com/recp/libgk/blob/master/src/scene/scene.c) and [accessing private fields](https://github.com/recp/libgk/blob/master/src/render/realtime/rn_scene.c#L31-L48) – recp Nov 27 '17 at 22:08
  • 1
    @dbush It's a hack, and it's also a *dangerous* hack. Any user who does `sizeof()` on the structure, or tries making an array of them, or performs arithmetic on a pointer to the structure is in for a nasty surprise. Either make the entire structure opaque and inaccessible directly, make it entirely open and eliminate the need for a private allocator, or use a compound structure - public with a pointer to an opaque structure. Hiding an opaque portion after the public portion is adding needless limitations that open up the door for disastrous bugs should a caller violate them. – Andrew Henle Nov 27 '17 at 22:45
  • @AndrewHenle it may dangerous but hack? Since the struct has custom allocator, how can users create array of them, skip the allocator and initialize it statically? Maybe I should create custom sizeof then it would fix all sizeof() and pointer arithmetics. I may put private fields back to public header as previous but I'm not sure, also making it inaccessible completely would brings lot of function calls for accessing all members (plus also validating parameters). It is not just limitations, it makes public headers more clear and separate library-level private job from public api level – recp Nov 27 '17 at 23:20
  • @recp If you provide a full `struct Scene`, anyone can do `struct Scene *p = malloc( sizeof( *p ) );` or `struct Scene sceneArray[1234];`. Your private allocator can't prevent that. And given `struct Scene *p = privAllocator();` you can't prevent `p++;` either. Both of those will cause serious problems with your proposed solution. *Maybe I should create custom sizeof* How do you think you can do that? If you want to keep the public headers simple, it's harder to be simpler - and safer - than an opaque structure with getter and setter functions. – Andrew Henle Nov 27 '17 at 23:44
  • @AndrewHenle: Should implementations whose `stdio.h` includes a complete structure definition for `FILE` be defective because such a declaration would syntactically allow code like `FILE my_file = *fopen("fred","r"); fprintf(&my_file, "Hello\n");` even though its behavior may be totally meaningless? – supercat Dec 05 '17 at 23:50
1

The language which Dennis Ritchie designed and called C defined many behaviors in terms of physical storage layouts. This made Ritchie's language superior to many other existing languages for purposes like systems programming, since it meant that the language could be used to implement many kinds of data structures beyond those imagined by its creator.

If a compiler makes a bona fide effort to support the semantics of Dennis Ritchie's language, it should have no trouble supporting the constructs you describe. Unfortunately, when writing a rule that was (according to the published rationale) supposed to allow compilers to make certain optimizations in most cases where there would be no reason to expect that they would change a program's semantics, they wrote it in a way that compiler writers have regarded as an invitation to ignore obvious evidence that such optimizations would change program semantics.

If there is no need to have "user" code see a completed structure type, it's possible to have a header file simply say typedef struct tagName typeName; and then use pass around pointers of type typeName*. Unfortunately, it may sometimes be necessary to do things that would require that a full type be available even if user code wouldn't need to actually access most of the members thereof. If that is necessary, I would suggest that it is better to use whatever compiler option(s) would be necessary to tell a compiler that it should allow for the possibility that a pointer of one structure type may be used to access members of another which has a suitable layout [-fno-strict-aliasing on gcc or clang], than to work around compiler writers' desire to process a gutted version of the language with such useful abilities removed.

supercat
  • 77,689
  • 9
  • 166
  • 211