[PATCH] errormsg: use descriptive macros for error numbers

[Steve Grubb]

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.

> 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.

> 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.

> > 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.

> 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.

> 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.

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.  :-)

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.

-Steve