1

This is in regards to some old (pre-C89) code that I'm working on. Part of that code is a small library that defines these structures in the header file:

// lib.h

struct data_node {
  const struct data_node *next;
  const struct data_node *prev;
  void *data;
};

struct trace_node {
  const struct trace_node *next;
  const struct trace_node *prev;
  unsigned int id;
  const char *file;
  int line;
};

const struct trace_node *get_trace(void);

The source file redefines those same structures, like so:

// lib.c
// does *not* include "lib.h"

struct data_node {
  struct data_node *next;
  struct data_node *prev;
  void *data;
};

struct trace_node {
  struct trace_node *next;
  struct trace_node *prev;
  unsigned int id;
  const char *file;
  int line;
  struct data_node *syncData; /* not included in header file version */
};

It works like you would expect: the syncData field is not visible to client code that includes the "lib.h" header.

Background

The library maintains 2 internal lists: the trace list, and the data list. The syncData field keeps the 2 lists in sync (go figure).

If client code had access to the syncData field, it could disrupt the synchronization between the lists. But, the trace list can get pretty large, so rather than copying every node into a smaller version of the struct, it just returns the address of the sentinel node for the internal list.

Question

I've compiled this with -Wall, -Wpedantic, and -Wextra, and I can't get gcc to complain about it, both with -std=c99 and -std=c11. A hex dump of the memory shows the bytes for the hidden field, right where they ought to be.

The relevant section of the standard (6.2.7.1) says:

Two types have compatible type if their types are the same. Additional rules for determining whether two types are compatible are described in 6.7.2 for type specifiers, in 6.7.3 for type qualifiers, and in 6.7.5 for declarators.46) Moreover, two structure, union, or enumerated types declared in separate translation units are compatible if their tags and members satisfy the following requirements: If one is declared with a tag, the other shall be declared with the same tag. If both are complete types, then the following additional requirements apply: there shall be a one-to-one correspondence between their members such that each pair of corresponding members are declared with compatible types, and such that if one member of a corresponding pair is declared with a name, the other member is declared with the same name. For two structures, corresponding members shall be declared in the same order. For two structures or unions, corresponding bit-fields shall have the same widths. For two enumerations, corresponding members shall have the same values.

Which, depending on how you want to read it, could be taken to say that compatible struct definitions are restricted to ONLY having corresponding pairs of members (and no others), or that struct definitions are compatible if, where they do have corresponding pairs of members, those pairs meet the requirements.

I don't think this is Undefined Behavior. At worst, I think it may be unspecified. Should I refactor this to use 2 distinct struct definitions? Doing this would require a performance hit to allocate a new public node for each node in the internal list and copy over the public data.

Mark Benningfield
  • 2,800
  • 9
  • 31
  • 31
  • There's only one way to read a *"a one-to-one correspondence"*. – StoryTeller - Unslander Monica Jan 22 '18 at 21:50
  • I don't understand how declaring some structs in a shared header file might impact any performance. – Eugene Sh. Jan 22 '18 at 22:01
  • @EugeneSh.: Edited to make it clearer that it would involve allocation and copying from one type of node to the other – Mark Benningfield Jan 22 '18 at 22:07
  • A **pair** is a pair - Once one half of the pair is missing, you violate the standard. If you want to do stuff like that, expose an anonymous structure pointer and accessors to the members that clients are allowed to access. – tofro Jan 22 '18 at 22:12
  • There *are* some guarantees about structs that share common initial subsets of members. See for example [this question](https://stackoverflow.com/questions/27889199/union-of-structs-sharing-same-first-members). – Steve Summit Jan 22 '18 at 22:16
  • @SteveSummit: That is for structures in a union together. That is not relevant here. – Eric Postpischil Jan 22 '18 at 22:19

4 Answers4

3

This has undefined behavior, and for good reasons.

First, the text clearly states that compatible struct must have a one-to-one correspondance between fields. So the behavior is undefined if client and library access the same object. A compiler can't detect that undefined behavior, because this rule is about stiching together knowledge from two different translation units that are compiled separately. This is why you don't see any diagnostic.

The reason that your example is particularly bad practise is that not even the sizes of the two struct types aggree and maybe not even their alignment. So a client that accesses such an object will make false assumptions about optimization opportunities.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
  • That's the part I wasn't quite sure of. There is so much qualifying text about the nature of the correspondence. It would have been much simpler, IMO, to say "A strict one-to-one correspondence of members", and leave it at that. – Mark Benningfield Jan 22 '18 at 22:13
  • @MarkBenningfield, improvement of these texts is always possible, but I don't see what we could easily leave out. For example, the parts that constrain the naming of tags and fields are important, in particular when link time optimization is more and more common. – Jens Gustedt Jan 23 '18 at 07:57
0

If lib.c does not include lib.h then definitions from there are not visible to lib.c so no conflicts.

C does not use function overloading and so it does not use name mangling and so if you have this declaration:

struct trace_node *get_trace(void);

in one place but function is implemented as

struct foo_trace_node *get_trace(void);

then the linker will happily link your code with that get_trace()

c-smile
  • 26,734
  • 7
  • 59
  • 86
  • It's still undefined behaviour however, which is what OP was asking – M.M Jan 22 '18 at 22:10
  • Even name mangling wouldn't help. Both sides use the same name for the `struct`, so even functions with the `struct` as argument would mangle the same. – Jens Gustedt Jan 22 '18 at 22:12
  • @M.M What about `void *get_trace(void);` declaration then? Will it be considered as undefined behavior? – c-smile Jan 22 '18 at 22:30
  • Yes, the type of the function being called must match the type the function is defined as – M.M Jan 22 '18 at 22:35
0

What you do is directly violating standards. A "one-to-one correspondance" would expect the pointer to be seen by client code as well. Your code violates the first part of that sentence.

Imagine the client code to unlink one structure from the list and create and link in one of "his" structures with the wrong size - Lib would crash sooner or later when dereferencing that pointer.

If you don't want to expose certain fields of structures, don't expose the structure at all. Hand the client code an anonymous structure pointer and expose accessor functions in the lib that return field values the client is allowed to see. Or pack the "allowed part" into an embedded structure within the larger one and hand that to client code if you want to avoid field-by-field access. It's probably also not a good idea to have client code see the link pointers of your list structure.

tofro
  • 5,640
  • 14
  • 31
  • Well, the list wouldn't be much good to the client if they couldn't iterate it. The `get_trace()` returns the sentinel node. – Mark Benningfield Jan 22 '18 at 22:30
  • There's not much (possibly none at all) performance lost exposing a library function `next(struct trace_node*)`. In a commercial library, I wouldn't expose such pointers and let clients mess with them. – tofro Jan 22 '18 at 22:38
0

I found some additional information in

The New C Standard
An Economic and Cultural Commentary
Derek M. Jones
derek@knosof.co.uk
http://www.coding-guidelines.com/cbook/cbook1_2.pdf

This particular version covers the C99 standard, but since the text for the relevant section is identical in both versions of the standard, it's a wash.

Pertinent commentary:

633
Moreover, two structure, union, or enumerated types declared in separate translation units are compatible if their tags and members satisfy the following requirements:

Commentary

These requirements apply if the structure or union type was declared via a typedef or through any other means. Because there can be more than one declaration of a type in the same translation unit, these requirements really apply to the composite type in each translation unit. In the following list of requirements, those that only apply to structures, unions, enumerations, or a combination thereof are explicitly called out as such. Two types are compatible if they obey both of the following requirements:

• Tag compatibility.

  1. If both types have tags, both shall be the same.
  2. If one, or neither, type has a tag, there is no requirement to be obeyed.

• Member compatibility.

Here the requirement is that for every member in both types there is a corresponding member in the other type that has the following properties:

  1. The corresponding members have a compatible type.
  2. The corresponding members either have the same name or are unnamed.
  3. For structure types, the corresponding members are defined in the same order in their respective definitions.
  4. For structure and union types, the corresponding members shall either both be bit-fields having the same width, or neither shall be bit-fields.
  5. For enumerated types, the corresponding members (the enumeration constants) have the same value.

So the verdict is unanimous. The wording in Mr. Jones' commentary is perhaps a little bit clearer (to me, at any rate).

I failed to mention in the OP that the original header file comments for the get_trace() function clearly state that the trace list is to be considered strictly read-only, so the points raised about an object of the abbreviated structure in client code finding its way back into the library code are - while still valid in the general sense - not exactly applicable in this specific case.

However, the question of compiler optimizations is bang on, especially considering how much more aggressive compiler optimizations are now, versus 35 years ago. So, I will refactor.

Mark Benningfield
  • 2,800
  • 9
  • 31
  • 31