5:36 a.m.
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(-)
diff --git a/lib/errormsg.h b/lib/errormsg.h
index 9524697..17ff767 100644
--- a/lib/errormsg.h
+++ b/lib/errormsg.h
@@ -68,4 +68,33 @@ static const struct msg_tab err_msgtab[] = {
{ -30, 2, "Field option not supported by kernel:" },
{ -31, 1, "must be used with exclude, user, or exit filter" },
};
+#define EAU_OPMISSING 1
+#define EAU_FIELDUNKNOWN 2
+#define EAU_ARCHMISPLACED 3
+#define EAU_ARCHUNKNOWN 4
+#define EAU_ELFUNKNOWN 5
+#define EAU_ARCHNOBIT 6
+#define EAU_EXITONLY 7
+#define EAU_MSGTYPEUNKNOWN 8
+#define EAU_MSGTYPEEXCLUDE 9
+#define EAU_UPGRADEFAIL 10
+#define EAU_STRTOOLONG 11
+#define EAU_MSGTYPECREDEXCLUDE 12
+#define EAU_OPEQNOTEQ 13
+#define EAU_PERMRWXA 14
+#define EAU_ERRUNKNOWN 15
+#define EAU_FILETYPEUNKNOWN 16
+#define EAU_EXITENTRYONLY 17
+#define EAU_KEYDEP 19
+#define EAU_FIELDVALMISSING 20
+#define EAU_FIELDVALNUM 21
+#define EAU_FIELDNAME 22
+#define EAU_COMPFIELDNAME 24
+#define EAU_COMPVAL 25
+#define EAU_COMPFIELDUNKNOWN 26
+#define EAU_COMPVALUNKNOWN 27
+#define EAU_FIELDTOOMANY 28
+#define EAU_OPEQ 29
+#define EAU_FIELDNOSUPPORT 30
+#define EAU_FIELDNOFILTER 31
#endif
diff --git a/lib/libaudit.c b/lib/libaudit.c
index 002dd89..a3b4261 100644
--- a/lib/libaudit.c
+++ b/lib/libaudit.c
@@ -520,7 +520,7 @@ int audit_reset_lost(int fd)
struct audit_status s;
if ((audit_get_features() & AUDIT_FEATURE_BITMAP_LOST_RESET) == 0)
- return -30;
+ return -EAU_FIELDNOSUPPORT;
memset(&s, 0, sizeof(s));
s.mask = AUDIT_STATUS_LOST;
@@ -979,7 +979,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
return -1;
if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
- return -28;
+ return -EAU_FIELDTOOMANY;
/* look for 2-char operators first
then look for 1-char operators afterwards
@@ -994,24 +994,24 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
*v++ = '\0';
op = AUDIT_EQUAL;
} else {
- return -13;
+ return -EAU_OPEQNOTEQ;
}
if (*f == 0)
- return -24;
+ return -EAU_COMPFIELDNAME;
if (*v == 0)
- return -25;
+ return -EAU_COMPVAL;
if ((field1 = audit_name_to_field(f)) < 0)
- return -26;
+ return -EAU_COMPFIELDUNKNOWN;
if ((field2 = audit_name_to_field(v)) < 0)
- return -27;
+ return -EAU_COMPVALUNKNOWN;
/* Interfield comparison can only be in exit filter */
if (flags != AUDIT_FILTER_EXIT)
- return -7;
+ return -EAU_EXITONLY;
// It should always be AUDIT_FIELD_COMPARE
rule->fields[rule->field_count] = AUDIT_FIELD_COMPARE;
@@ -1392,7 +1392,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const
char *pair,
return -1;
if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
- return -28;
+ return -EAU_FIELDTOOMANY;
/* look for 2-char operators first
then look for 1-char operators afterwards
@@ -1430,23 +1430,23 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
}
if (v == NULL)
- return -1;
+ return -EAU_OPMISSING;
if (*f == 0)
- return -22;
+ return -EAU_FIELDNAME;
if (*v == 0)
- return -20;
+ return -EAU_FIELDVALMISSING;
if ((field = audit_name_to_field(f)) < 0)
- return -2;
+ return -EAU_FIELDUNKNOWN;
/* Exclude filter can be used only with MSGTYPE and cred fields */
if (flags == AUDIT_FILTER_EXCLUDE) {
uint32_t features = audit_get_features();
if ((features & AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND) == 0) {
if (field != AUDIT_MSGTYPE)
- return -30;
+ return -EAU_FIELDNOSUPPORT;
} else {
switch(field) {
case AUDIT_PID:
@@ -1461,7 +1461,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const
char *pair,
case AUDIT_SUBJ_CLR:
break;
default:
- return -12;
+ return -EAU_MSGTYPECREDEXCLUDE;
}
}
}
@@ -1529,13 +1529,13 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
rule->values[rule->field_count] =
audit_name_to_errno(v);
if (rule->values[rule->field_count] == 0)
- return -15;
+ return -EAU_ERRUNKNOWN;
}
break;
case AUDIT_MSGTYPE:
if (flags != AUDIT_FILTER_EXCLUDE &&
flags != AUDIT_FILTER_USER)
- return -9;
+ return -EAU_MSGTYPEEXCLUDE;
if (isdigit((char)*(v)))
rule->values[rule->field_count] =
@@ -1545,7 +1545,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const
char *pair,
rule->values[rule->field_count] =
audit_name_to_msg_type(v);
else
- return -8;
+ return -EAU_MSGTYPEUNKNOWN;
break;
/* These next few are strings */
case AUDIT_OBJ_USER:
@@ -1558,7 +1558,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const
char *pair,
/* Watch & object filtering is invalid on anything
* but exit */
if (flags != AUDIT_FILTER_EXIT)
- return -7;
+ return -EAU_EXITONLY;
if (field == AUDIT_WATCH || field == AUDIT_DIR)
_audit_permadded = 1;
@@ -1573,21 +1573,21 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
if (field == AUDIT_EXE) {
uint32_t features = audit_get_features();
if ((features & AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH) == 0)
- return -30;
+ return -EAU_FIELDNOSUPPORT;
if (op != AUDIT_EQUAL)
- return -29;
+ return -EAU_OPEQ;
_audit_exeadded = 1;
}
if (field == AUDIT_FILTERKEY &&
!(_audit_syscalladded || _audit_permadded ||
_audit_exeadded))
- return -19;
+ return -EAU_KEYDEP;
vlen = strlen(v);
if (field == AUDIT_FILTERKEY &&
vlen > AUDIT_MAX_KEY_LEN)
- return -11;
+ return -EAU_STRTOOLONG;
else if (vlen > PATH_MAX)
- return -11;
+ return -EAU_STRTOOLONG;
rule->values[rule->field_count] = vlen;
offset = rule->buflen;
rule->buflen += vlen;
@@ -1604,21 +1604,21 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
break;
case AUDIT_ARCH:
if (_audit_syscalladded)
- return -3;
+ return -EAU_ARCHMISPLACED;
if (!(op == AUDIT_NOT_EQUAL || op == AUDIT_EQUAL))
- return -13;
+ return -EAU_OPEQNOTEQ;
if (isdigit((char)*(v))) {
int machine;
errno = 0;
_audit_elf = strtoul(v, NULL, 0);
if (errno)
- return -5;
+ return -EAU_ELFUNKNOWN;
// Make sure we have a valid mapping
machine = audit_elf_to_machine(_audit_elf);
if (machine < 0)
- return -5;
+ return -EAU_ELFUNKNOWN;
}
else {
const char *arch=v;
@@ -1628,7 +1628,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const
char *pair,
to elf. */
elf = audit_machine_to_elf(machine);
if (elf == 0)
- return -5;
+ return -EAU_ELFUNKNOWN;
_audit_elf = elf;
}
@@ -1637,15 +1637,15 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
break;
case AUDIT_PERM:
if (flags != AUDIT_FILTER_EXIT)
- return -7;
+ return -EAU_EXITONLY;
else if (op != AUDIT_EQUAL)
- return -13;
+ return -EAU_OPEQNOTEQ;
else {
unsigned int i, len, val = 0;
len = strlen(v);
if (len > 4)
- return -11;
+ return -EAU_STRTOOLONG;
for (i = 0; i < len; i++) {
switch (tolower(v[i])) {
@@ -1662,7 +1662,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const
char *pair,
val |= AUDIT_PERM_ATTR;
break;
default:
- return -14;
+ return -EAU_PERMRWXA;
}
}
rule->values[rule->field_count] = val;
@@ -1670,11 +1670,11 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
break;
case AUDIT_FILETYPE:
if (!(flags == AUDIT_FILTER_EXIT))
- return -17;
+ return -EAU_EXITENTRYONLY;
rule->values[rule->field_count] =
audit_name_to_ftype(v);
if ((int)rule->values[rule->field_count] < 0) {
- return -16;
+ return -EAU_FILETYPEUNKNOWN;
}
break;
case AUDIT_ARG0...AUDIT_ARG3:
@@ -1687,16 +1687,16 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
rule->values[rule->field_count] =
strtol(v, NULL, 0);
else
- return -21;
+ return -EAU_FIELDVALNUM;
break;
case AUDIT_SESSIONID:
if ((audit_get_features() &
AUDIT_FEATURE_BITMAP_SESSIONID_FILTER) == 0)
- return -30;
+ return -EAU_FIELDNOSUPPORT;
if (flags != AUDIT_FILTER_EXCLUDE &&
flags != AUDIT_FILTER_USER &&
flags != AUDIT_FILTER_EXIT)
- return -31;
+ return -EAU_FIELDNOFILTER;
// Do positive & negative separate for 32 bit systems
vlen = strlen(v);
if (isdigit((char)*(v)))
@@ -1712,20 +1712,20 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep,
const char *pair,
case AUDIT_DEVMAJOR...AUDIT_INODE:
case AUDIT_SUCCESS:
if (flags != AUDIT_FILTER_EXIT)
- return -7;
+ return -EAU_EXITONLY;
/* fallthrough */
default:
if (field == AUDIT_INODE) {
if (!(op == AUDIT_NOT_EQUAL ||
op == AUDIT_EQUAL))
- return -13;
+ return -EAU_OPEQNOTEQ;
}
if (field == AUDIT_PPID && !(flags==AUDIT_FILTER_EXIT))
- return -17;
+ return -EAU_EXITENTRYONLY;
if (!isdigit((char)*(v)))
- return -21;
+ return -EAU_FIELDVALNUM;
if (field == AUDIT_INODE)
rule->values[rule->field_count] =
--
1.7.1
4:33 p.m.
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. But we still have hardcoded numbers in the err_msgtab[]. :-)
Thanks,
-Steve
1:39 a.m.
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? 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. 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.
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. 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? Can we
create some more and start using that facility for items in your TODO?
-Steve
- RGB
--
Richard Guy Briggs <rgb(a)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
5:14 p.m.
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
3:42 a.m.
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(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.
"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(a)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
8:32 p.m.
On 2017-04-13 04:42, Richard Guy Briggs wrote:
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(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.
"git am -s <mbox>|<Maildir>" will do that and add your signature.
I should add, "git am -s --reject <mbox>" will apply the hunks it can
and leave you with .rej files that make it much easier to deal with
patches that don't apply cleanly rather than just saying it failed
(though I don't need to tell you that you would be in your rights to
simply reject the patch if it doesn't apply cleanly, requesting the
submitter to use an updated tree).
> > 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(a)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
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rgb(a)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
2776
days inactive
2819
days old
linux-audit@lists.linux-audit.osci.io
5 comments
2 participants
participants (2)
-
Richard Guy Briggs -
Steve Grubb