[PATCH 02/14] Add TSEM specific documentation.

[Paul Moore]

On Wed, Mar 29, 2023 at 11:35 PM Dr. Greg <greg at 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 at 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.

> 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

-- 
paul-moore.com