Re: [PATCH] errormsg: use descriptive macros for error numbers
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(a)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