Re: [PATCH 02/14] Add TSEM specific documentation.

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

On Wed, Apr 05, 2023 at 04:45:26PM -0400, Paul Moore wrote:

Good morning, I hope the week has gone well for everyone.

> On Wed, Mar 29, 2023 at 11:35???PM Dr. Greg <greg@enjellic.com> wrote:
> > On Wed, Mar 22, 2023 at 07:45:26PM -0400, Paul Moore wrote:
> > > On Mon, Mar 13, 2023 at 6:52???PM Dr. Greg <greg@enjellic.com> wrote:
> > > > On Thu, Mar 02, 2023 at 11:15:56PM -0500, Paul Moore wrote:
> > > >
> > > > Hi Paul, thanks for sending along further comments.
> > > >
> > > > You note below that you haven't had time to look at the code since you
> > > > wanted to confirm the TSEM security model before moving forward.
> > > >
> > > > From a development perspective we are now three weeks into what will
> > > > become version 2 of the patch series.  So at this point I wouldn't
> > > > advocate spending a lot of time on the current patchset.
> > > >
> > > > That being said, if you some have time, we would appreciate a quick
> > > > look at the code on your part, with respect to style changes and the
> > > > like we can enforce in the second series, ie. ordering of local
> > > > variable declarations by length and the like.
> >
> > > To be perfectly honest I'm still very concerned about some of the
> > > issues that I've seen in the docs, until that is sorted out I'm not
> > > sure there is much point in looking at the code.
> >
> > I think those concerns can be resolved, see below for more
> > information, the second patch series that we are closing in on should
> > help address the concerns that are currently on the table.

> In that case, I think it might be best to wrap up this thread and we
> can resume the discussion on the next patchset.

Very good, we will look forward to the review of V2, which has now
been enhanced on a number of fronts.

> > That being said, since TSEM is a new codebase, we were hoping that you
> > could give us some guidance on function local variable ordering.
> > Reverse Christmas tree seems popular writ large in the kernel, I
> > believe that you commented in a posting a month or two ago that you
> > prefer standard Christmas tree, SMACK and SeLinux don't seem to
> > religiously embrace a style.
> >
> > Our codebase uses ordering based on least complex to most complex
> > variables and has worked for us, both in the kernel and elsewhere, but
> > we are ambivalent, as our primary objective is to avoid wasting
> > everyone's time on issues such as this.
> 
> The canonical guidance on coding style within the kernel is in the kernel docs:
> 
> https://www.kernel.org/doc/html/latest/process/coding-style.html
> 
> When in doubt, I would recommend following that as closely as possible.
> 
> As far as local variable ordering is concerned, I don't believe I've
> ever rejected patches due to that.  My own personal preference usually
> follows what you've described above: the least complex (simple
> scalars) at the top, with the more complex variables (composites) at
> the bottom.  In practice this tends to result in a "Christmas Tree"
> ordering, but it can be a bit lumpy (?) in some cases; that is fine.
> 
> There are two style nitpicks which annoy me enough to make it worth mentioning:
> 
> * Stick to 80 characters as much as possible, yes we all have
> terminals that can go wider, but I like to have several terminals on
> my screen and if they all need to be 100 chars wide I can't fit as
> many.  There are going to be some exceptions, e.g. error message
> string literals, but that should only happen a few times in a given
> file.  If you are finding that every function you write has a line
> that goes past 80 characters you are doing *something* wrong.
> 
> * If/when you split a single line across multiple lines due to the
> above, make sure the
> lines are indented properly such that they line up properly with the
> code above.  It's tricky to give all of the different examples so I'm
> not going to even try.  I realize that is garbage guidance, but the
> kernel coding style is a help here.
> 
> If you are really lost, I use the following 'astyle' command in other
> projects, and it should produce fairly kernel-style friendly code:
> 
> % astyle --options=none --lineend=linux --mode=c \
>     --style=linux \
>     --indent=force-tab=8 \
>     --indent-preprocessor \
>     --indent-col1-comments \
>     --min-conditional-indent=0 \
>     --max-instatement-indent=80 \
>     --pad-oper \
>     --align-pointer=name \
>     --align-reference=name \
>     --max-code-length=80 \
>     --break-after-logical

All of the above is consistent with what we have used for years,
particularly 80 columns, given that the principles of TSEM extend from
programming with a Model 29 keypunch and a drum card to applying
machine learning to security.

> paul-moore.com

Have a good weekend.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity