[Swan-dev] [Swan-commit] modularity erosion

[D. Hugh Redelmeier]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| /*
|  * XXX: useful?
|  */
| struct ike_proposals {
|         struct proposals *p;
| };
| struct child_proposals {
|         struct proposals *p;
| };

This is a nice safety measure/trick.  Not necessarily in this case (I have
no opinion), but as one of our bag of C tools for making clearer and
safer code.

To be clear:

- you can wrap a somewhat generic datastructure in a unique struct
  type so that the languages type rules prevent you from using the
  wrapped type in inappropriate places.

Toy Example:

	struct inches {
		int in;
	};

	struct metres {
		int m;
	};

This will make it awkward to use inches where metres are expected.

Unfortunately, it is often a bit cumbersome.

| A quick examination of the rewritten proposal code, including
| algparse, shows nothing is making use of these declarations.  Instead
| 'struct proposals' is opaque (per #1) and the header wrapped in #ifdef
| (per #3).  FWIW, much of the new code follows this style (the
| exception being some primitive types like monotime_t et.al.).
|
| However, pluto does still use the declarations - they provide a way to
| strongly differentiate between an IKE and CHILD proposal.  But that is
| something local to pluto and connections.h, hence my note above
| pointing out that it should be moved as a *cleanup*.

(I know you know this.)

The (generic) trouble is that C only allows pointer-to-struct fields
to be opaque, not struct fields.  The compiler needs to know the size
of the field; all possible struct pointers are the same size, but not
all possible structs.

On the other hand, memory management is sometimes more onerous
pointers-to-structs.  Sub-structs are automatically included in
structs.

Making a wise trade-off between these issues is part of the job of a C
programmer.

| But "XXX: useful?"?  Well, when rewriting the parser I did consider
| dumping those two declarations and instead just using 'struct
| proposals *' in "connections.h".  I decided against it.  It seemed to
| me that the stronger type helped with readability vis:
|    ikev1_quick_pfs(const struct child_proposals proposals)

The question I tried to raise (less concretely) was
	struct ike_proposals vs struct ike_proposals *
not
	struct ike_proposals vs struct proposals

Both are interesting question.

But I was just hoping that we could make the type opaque to the code
that has no Need to Know.  I admit that the cost might be too high.

More generally, we are constantly increasing the entropy of our code
and I want us to push back at that.