svn commit: r316182 - in head/sys: compat/freebsd32 kern sys

[Konstantin Belousov]

On Thu, Mar 30, 2017 at 09:22:07AM +0100, Robert Watson wrote:
> On Thu, 30 Mar 2017, Konstantin Belousov wrote:
> 
> >>   Hook up new audit event identifiers for various non-Orange Book/CAPP
> >>   system calls supported by OpenBSM 1.2-alpha5.
> >>
> >>   Obtained from:	TrustedBSD Project
> >>   MFC after:	3 weeks
> >>   Sponsored by:	DARPA, AFRL
> >>
> >> Modified:
> >>   head/sys/compat/freebsd32/freebsd32_proto.h
> >>   head/sys/compat/freebsd32/freebsd32_sysent.c
> >>   head/sys/compat/freebsd32/syscalls.master
> >>   head/sys/kern/init_sysent.c
> >>   head/sys/kern/syscalls.master
> >>   head/sys/sys/sysproto.h
> >
> > This was already discussed somewhere, might be the place was not public 
> > enough.
> >
> > The change, as committed, is impossible to read. Please do not mix 
> > human-made changes and generated files in one commit. For head it is no much 
> > harm to split the syscalls.master commits in way it was always done, for 
> > sake of the people reading either commit mails or VCS diffs.
> >
> > I have no opinion on the split/single commit on stable branches.
> 
> Hi Kostik:
> 
> My apologies -- I'm afriad I lost track of that convention during
> the merge. I have to say, though, that if we want to make changes
> easier to follow, the problem here is not so much generated files as
> poor formatting in generated files. Reading syscalls.master changes
> is always hard when multiple system calls are affected, due to the
> choice of putting all metadata for a system call on one line in the
> file. We could fix this in the generated files easily, though, by
> having the structs in init_sysent.c add carriage returns after each
> field assignment -- in which case the generated changes would make
> it easier to understand what was going on for multi-syscall commits,
> and help catch errors better. (I.e., by having the assignment of
> various system-call-related fields in the struct each appear on their
> own line.) At which point I suspect I'd generally prefer to see the
> commits combined. (The other reason we used to not combine commits
> related to generated $FreeBSD$ IDs in the files .. but that seems to
> have gone away.

The generated files diff would still be in the diff.  Reading this diff
only makes sense when a bug in makesyscalls.sh is looked for.  So the diff
still wastes the time of the reader, even if microoptimized to reduce
the size.

IMO the solution there is to start auto-generating the syscall tables
from metadata, similarly to vnode_if.src. Of course, the complication
there is that the generated files are needed for installincludes
and libc/libthr build. Until this is done, I would prefer to have
the generated files to not contaminate human-readable diff with
auto-generated changes.