[PATCH] errormsg: use descriptive macros for error numbers

Thu Apr 13 08:42:29 UTC 2017

On 2017-04-12 18:14, Steve Grubb wrote:
> On Wednesday, April 12, 2017 2:39:26 AM EDT Richard Guy Briggs wrote:
> > On 2017-04-11 17:33, Steve Grubb wrote:
> > > On Tuesday, April 4, 2017 6:36:52 AM EDT Richard Guy Briggs wrote:
> > > > Convert all the numerical error return codes in comparison option and
> > > > field option parsing routines audit_rule_interfield_comp_data() and
> > > > audit_rule_fieldpair_data() to descriptive macros for easier code
> > > > navigation and verification.
> > > > 
> > > > See: https://github.com/linux-audit/audit-userspace/issues/11
> > > > 
> > > > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> > > > ---
> > > > 
> > > >  lib/errormsg.h |   29 +++++++++++++++++++
> > > >  lib/libaudit.c |   84
> > > > 
> > > > ++++++++++++++++++++++++++++---------------------------- 2 files
> > > > changed,
> > > > 71 insertions(+), 42 deletions(-)
> > > 
> > > Applied.
> > 
> > Thanks for taking this patch.  Are you able to adapt your workflow so
> > that the original author of contributed patches shows up in the git
> > commit author line with the original patch timestamp?
> 
> Not sure how that works. patch -p1 < email.mbox doesn't really allow for that.

"git am -s <mbox>|<Maildir>" will do that and add your signature.

> > Looking back through the repo history it appears you are the only author
> > since you took over from mitr, which I know not to be true.
> 
> I give attribution in the Changelog when the patch fixes a bug or adds a 
> feature. Spelling errors and shuffling code around usually don't get any 
> mention. If there is a big contribution, I put something in the THANKS file. 
> This project has lasted longer that git or github has been around. I'm 
> following the GNITS standard.

If you use git am instead, it'll be self-documenting.  I seem to recall
svn had similar facilities.

Linus has been doing this since v2.5.3 in 2002 through at least one SCM
system change.  I'm pretty sure he had that history before that too for
GPL copyright licence accountability, but it was too painful to port to
the new SCM adopted in 2002.  The recent legal copyright licence query
about a few files in the audit userspace package would have been easier
to answer with that automation.

> > As it is now, you appear to be the author of this patch and the patch date
> > is a week later than it actually was.  There is no way to the original
> > author now.  "git am" is able to do this.
> 
> Is this really a problem? I have sent hundreds of lines of fixes to shadow-
> utils and I honestly don't care what's in their git logs or even care if they 
> use git. This project has always used the Changelog and THANKS files for 
> attribution. And they survive moving to another SCCS if we decide to do that.

It is when people are looking for automated statistics.  I see no need
to change the use of the Changelog and THANKS files.

> > > But we still have hardcoded numbers in the err_msgtab[].  :-)
> > 
> > Yes.  Once all the return codes that use this function have been
> > accounted for and have been converted to macros, we can throw the macros
> > into an enum and not care what their actual value is and then convert
> > the table.
> 
> Could have done that all in one shot. That would have been preferred. But 
> don't take this the wrong way. Thanks for your contribution.

There are still two more patches outstanding that were part of that
effort.  I split them up because the first step was more obvious and
made the other steps easier to understand.  I could have squashed all
three patches together, but I chose to leave them seperate to make them
easier to understand when being reviewed and accpted.  I figured there
was a greater chance of the three patches being accepted if it was clear
what each one did.  The other two have not yet been accepted, so that
suggests to me that they required more careful review since they changed
existing behaviour as opposed to this one that did not.

> > One step at a time!  I wasn't able to see any of the duplication,
> > overloading or message meanings drifting over time without
> > this first step.
> > 
> > 
> > Note: GitHub Milestones: I had arbitrarily picked the only listed
> > milestone in github (which seemed reasonable at the time) to see how
> > well that mechanism worked.  Was that an appropriate milestone? 
> 
> I have not looked. I do not use the milestones. Things happen. Plans change.

So who created that one existing milestone?

> > Can we create some more and start using that facility for items in your
> > TODO?
> 
> I don't want to make more work for myself. No one contributes anything that is 
> in the TODO except one time. (I have to give a shout out to Burn for tackling 
> the auparse lol conversion. That was a major step in reliability for auparse.) 
> But seriously, why should I make things harder on myself when there is not 
> much in the way of help? I also review and reprioritize things every now and 
> then. Having milestones would just make that messier. For example, two weeks 
> ago I did not know we'd be doing a 2.7.5 release this week.

It might just make things easier for others to participate.

> If people really would like to contribute, the most important thing right now 
> is cleaning up the events. The log normalizer makes this a much higher 
> priority because it shows exactly when events are needing to be fixed. No 
> milestones are needed. Patches are needed.  :-)

If there is a kernel github issue for each of the records that need
cleanup, they are on the list.

> And if anyone wants to contribute user space code, just ask on the list or 
> privately how you can help. I can pick something from the TODO that can be 
> simple or a medium sized chunk. I also expect a little discussion before doing 
> it to make sure the proposed patch is close to how I envisioned it.

It sounds so simple, but not everybody is comfortable asking how they
can help and just dive in to see how things work and start fixing or
building things.

> -Steve

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635