Re: [PATCH 05/14] Add TSEM master header file.

["Dr. Greg" <greg@enjellic.com>]

On Sat, Feb 04, 2023 at 06:58:08PM -0800, Casey Schaufler wrote:

> On 2/3/2023 9:09 PM, Dr. Greg wrote:
> > TSEM is designed, from a functional perspective, to be contained
> > entirely in its own directory.
> >
> > The tsem.h header file defines the enumeration types, structure
> > definitions and externally visiable functions that are referenced
> > by the TSEM LSM implementation.

Hi Casey, thanks for taking the time to look at all of this and raise
issues, we appreciate the feedback.

> This is really awkward.

With respect to awkward, those who know me well would never accuse me
of terpsichoric elegance, either balletic or in technology.... :-)

> Without the uses of the structures and constants it's very difficult
> to understand how any of this makes sense. It would be easier to
> review if the structures and constants were introduced as they are
> used rather than being presented in one massive dump.

With respect to any of this making sense, we've read and studied the
existing LSM's fairly extensively as we built TSEM, we accept our
prejudices of course, but we think that TSEM is probably one of the
easiest to follow conceptually.

Doesn't mean we can't do better though, particularly as we look at the
issue of that header file retrospectively.

I think where the presentation of the structures and enumeration
types, as a single entity falls down, is not explicitly documenting
the role and function of the structures and constants in that file.
We did that for the external functions in the compilation units and
should have done that for the contents of the include file.

I'm not sure that adding things to the include file incrementally will
be any more understandable without more explicit documentation.  If we
document them now, it should help not only the review process but
anyone who needs to look at the code down the road with a mind on
understanding whqt is going on.

Having the include file introduced as a standalone entity also assists
in reliably rolling a review patch set.  It is already easy enough to
make a mistake and we don't want to waste reviewer time and list
bandwidith with a blown patchset.

We will collect whatever additional comments are forthcoming and fully
document the contents of that file for the second spin of the patch.

Thanks again for your observations.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity