All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] errormsg: correct a number of messages that have drifted
@ 2017-04-04 10:37 Richard Guy Briggs
  2017-04-04 10:37 ` [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes Richard Guy Briggs
  2017-05-04 19:50 ` [PATCH 1/2] errormsg: correct a number of messages that have drifted Steve Grubb
  0 siblings, 2 replies; 14+ messages in thread
From: Richard Guy Briggs @ 2017-04-04 10:37 UTC (permalink / raw)
  To: linux-audit; +Cc: Richard Guy Briggs

A number of error message descriptions have drifted from the conditions that
caused them in audit_rule_fieldpair_data() including expansion of fields to be
used by the user filter list, restriction to the exit list only and changing an
operator to "equals" only.  Correct these, using the new errormsg macros.

See: https://github.com/linux-audit/audit-userspace/issues/12

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 lib/errormsg.h |    4 ++--
 lib/libaudit.c |   10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/errormsg.h b/lib/errormsg.h
index 17ff767..35b7f95 100644
--- a/lib/errormsg.h
+++ b/lib/errormsg.h
@@ -44,7 +44,7 @@ static const struct msg_tab err_msgtab[] = {
     { -6,    1,    "requested bit level not supported by machine" },
     { -7,    1,    "can only be used with exit filter list" },
     { -8,    2,    "-F unknown message type -" },
-    { -9,    0,    "msgtype field can only be used with exclude filter list" },
+    { -9,    0,    "msgtype field can only be used with exclude or user filter list" },
     { -10,    0,    "Failed upgrading rule" },
     { -11,    0,    "String value too long" },
     { -12,    0,    "Only msgtype, *uid, *gid, pid, and subj* fields can be used with exclude filter" },
@@ -76,7 +76,7 @@ static const struct msg_tab err_msgtab[] = {
 #define EAU_ARCHNOBIT		6
 #define EAU_EXITONLY		7
 #define EAU_MSGTYPEUNKNOWN	8
-#define EAU_MSGTYPEEXCLUDE	9
+#define EAU_MSGTYPEEXCLUDEUSER	9
 #define EAU_UPGRADEFAIL		10
 #define EAU_STRTOOLONG		11
 #define EAU_MSGTYPECREDEXCLUDE	12
diff --git a/lib/libaudit.c b/lib/libaudit.c
index a3b4261..b481f52 100644
--- a/lib/libaudit.c
+++ b/lib/libaudit.c
@@ -1516,7 +1516,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 			break;
 		case AUDIT_EXIT:
 			if (flags != AUDIT_FILTER_EXIT)
-				return -7;
+				return -EAU_EXITONLY;
 			vlen = strlen(v);
 			if (isdigit((char)*(v))) 
 				rule->values[rule->field_count] = 
@@ -1535,7 +1535,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 		case AUDIT_MSGTYPE:
 			if (flags != AUDIT_FILTER_EXCLUDE &&
 					flags != AUDIT_FILTER_USER)
-				return -EAU_MSGTYPEEXCLUDE;
+				return -EAU_MSGTYPEEXCLUDEUSER;
 
 			if (isdigit((char)*(v)))
 				rule->values[rule->field_count] =
@@ -1639,7 +1639,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 			if (flags != AUDIT_FILTER_EXIT)
 				return -EAU_EXITONLY;
 			else if (op != AUDIT_EQUAL)
-				return -EAU_OPEQNOTEQ;
+				return -EAU_OPEQ;
 			else {
 				unsigned int i, len, val = 0;
 
@@ -1670,7 +1670,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 			break;
 		case AUDIT_FILETYPE:
 			if (!(flags == AUDIT_FILTER_EXIT))
-				return -EAU_EXITENTRYONLY;
+				return -EAU_EXITONLY;
 			rule->values[rule->field_count] = 
 				audit_name_to_ftype(v);
 			if ((int)rule->values[rule->field_count] < 0) {
@@ -1722,7 +1722,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 			}
 
 			if (field == AUDIT_PPID && !(flags==AUDIT_FILTER_EXIT))
-				return -EAU_EXITENTRYONLY;
+				return -EAU_EXITONLY;
 			
 			if (!isdigit((char)*(v)))
 				return -EAU_FIELDVALNUM;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-04-04 10:37 [PATCH 1/2] errormsg: correct a number of messages that have drifted Richard Guy Briggs
@ 2017-04-04 10:37 ` Richard Guy Briggs
  2017-05-04 20:11   ` Steve Grubb
  2017-05-04 19:50 ` [PATCH 1/2] errormsg: correct a number of messages that have drifted Steve Grubb
  1 sibling, 1 reply; 14+ messages in thread
From: Richard Guy Briggs @ 2017-04-04 10:37 UTC (permalink / raw)
  To: linux-audit; +Cc: Richard Guy Briggs

Several return codes were overloaded and no longer giving helpful error
return messages from the field and comparison functions
audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().

Introduce 3 new macros with more helpful error descriptions for data
missing, incompatible fields and incompatible values.

See: https://github.com/linux-audit/audit-userspace/issues/12

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 lib/errormsg.h |    6 ++++++
 lib/libaudit.c |   28 ++++++++++++++--------------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/errormsg.h b/lib/errormsg.h
index 35b7f95..50c7d50 100644
--- a/lib/errormsg.h
+++ b/lib/errormsg.h
@@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
     { -29,    1,    "only takes = operator" },
     { -30,    2,    "Field option not supported by kernel:" },
     { -31,    1,    "must be used with exclude, user, or exit filter" },
+    { -32,    0,    "field data is missing" },
+    { -33,    2,    "-C field incompatible" },
+    { -34,    2,    "-C value incompatible" },
 };
 #define EAU_OPMISSING		1
 #define EAU_FIELDUNKNOWN	2
@@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
 #define EAU_OPEQ		29
 #define EAU_FIELDNOSUPPORT	30
 #define EAU_FIELDNOFILTER	31
+#define EAU_DATAMISSING		32
+#define EAU_COMPFIELDINCOMPAT	33
+#define EAU_COMPVALINCOMPAT	34
 #endif
diff --git a/lib/libaudit.c b/lib/libaudit.c
index b481f52..b1f8f9c 100644
--- a/lib/libaudit.c
+++ b/lib/libaudit.c
@@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 	struct audit_rule_data *rule = *rulep;
 
 	if (f == NULL)
-		return -1;
+		return -EAU_DATAMISSING;
 
 	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
 		return -EAU_FIELDTOOMANY;
@@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_UID_TO_EUID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_FSUID:
@@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_UID_TO_FSUID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_LOGINUID:
@@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_UID_TO_AUID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_SUID:
@@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_UID_TO_SUID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_OBJ_UID:
@@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_SUID_TO_OBJ_UID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_UID:
@@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_UID_TO_SUID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 
@@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_EGID_TO_SGID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_FSGID:
@@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						 AUDIT_COMPARE_EGID_TO_FSGID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_GID:
@@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_GID_TO_SGID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_OBJ_GID:
@@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_SGID_TO_OBJ_GID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		case AUDIT_SGID:
@@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct audit_rule_data **rulep,
 						AUDIT_COMPARE_EGID_TO_SGID;
 				break;
 			default:
-				return -1;
+				return -EAU_COMPVALINCOMPAT;
 			}
 			break;
 		default:
-			return -1;
+			return -EAU_COMPFIELDINCOMPAT;
 			break;
 	}
 	rule->field_count++;
@@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data **rulep, const char *pair,
 	struct audit_rule_data *rule = *rulep;
 
 	if (f == NULL)
-		return -1;
+		return -EAU_DATAMISSING;
 
 	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
 		return -EAU_FIELDTOOMANY;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] errormsg: correct a number of messages that have drifted
  2017-04-04 10:37 [PATCH 1/2] errormsg: correct a number of messages that have drifted Richard Guy Briggs
  2017-04-04 10:37 ` [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes Richard Guy Briggs
@ 2017-05-04 19:50 ` Steve Grubb
  2017-05-04 20:25   ` Richard Guy Briggs
  1 sibling, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2017-05-04 19:50 UTC (permalink / raw)
  To: Richard Guy Briggs, Paul Moore; +Cc: linux-audit

On Tuesday, April 4, 2017 6:37:47 AM EDT Richard Guy Briggs wrote:
> A number of error message descriptions have drifted from the conditions that
> caused them in audit_rule_fieldpair_data() including expansion of fields to
> be used by the user filter list, restriction to the exit list only and
> changing an operator to "equals" only.  Correct these, using the new
> errormsg macros.
> 
> See: https://github.com/linux-audit/audit-userspace/issues/12
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

This was hard to check because the first patch switch numbers to macros in 
libaudit.c, but did not change them in the errormsg tab. It would have been 
better to apply this first and then change to macros or change everything to 
macros and then do this last. In any event, it's applied.

-Steve

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-04-04 10:37 ` [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes Richard Guy Briggs
@ 2017-05-04 20:11   ` Steve Grubb
  2017-05-04 20:29     ` Richard Guy Briggs
  0 siblings, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2017-05-04 20:11 UTC (permalink / raw)
  To: Richard Guy Briggs, Paul Moore; +Cc: linux-audit

On Tuesday, April 4, 2017 6:37:48 AM EDT Richard Guy Briggs wrote:
> Several return codes were overloaded and no longer giving helpful error
> return messages from the field and comparison functions
> audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().
> 
> Introduce 3 new macros with more helpful error descriptions for data
> missing, incompatible fields and incompatible values.
> 
> See: https://github.com/linux-audit/audit-userspace/issues/12
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  lib/errormsg.h |    6 ++++++
>  lib/libaudit.c |   28 ++++++++++++++--------------
>  2 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/errormsg.h b/lib/errormsg.h
> index 35b7f95..50c7d50 100644
> --- a/lib/errormsg.h
> +++ b/lib/errormsg.h
> @@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
>      { -29,    1,    "only takes = operator" },
>      { -30,    2,    "Field option not supported by kernel:" },
>      { -31,    1,    "must be used with exclude, user, or exit filter" },
> +    { -32,    0,    "field data is missing" },

Actually, this means that the filter is missing in the rule. This is the kind 
of thing I would normally just fixup after patching the source.

> +    { -33,    2,    "-C field incompatible" },
> +    { -34,    2,    "-C value incompatible" },
>  };
>  #define EAU_OPMISSING		1
>  #define EAU_FIELDUNKNOWN	2
> @@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
>  #define EAU_OPEQ		29
>  #define EAU_FIELDNOSUPPORT	30
>  #define EAU_FIELDNOFILTER	31
> +#define EAU_DATAMISSING		32
> +#define EAU_COMPFIELDINCOMPAT	33
> +#define EAU_COMPVALINCOMPAT	34
>  #endif
> diff --git a/lib/libaudit.c b/lib/libaudit.c
> index b481f52..b1f8f9c 100644
> --- a/lib/libaudit.c
> +++ b/lib/libaudit.c
> @@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, struct audit_rule_data *rule = *rulep;
> 
>  	if (f == NULL)
> -		return -1;
> +		return -EAU_DATAMISSING;
> 
>  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
>  		return -EAU_FIELDTOOMANY;
> @@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_EUID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;

This means that we are attempting an incompatible comparison between fields.

>  			}
>  			break;
>  		case AUDIT_FSUID:
> @@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_FSUID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_LOGINUID:
> @@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_AUID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_SUID:
> @@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_OBJ_UID:
> @@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_SUID_TO_OBJ_UID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_UID:
> @@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
> 
> @@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_FSGID:
> @@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_FSGID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_GID:
> @@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_GID_TO_SGID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_OBJ_GID:
> @@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_SGID_TO_OBJ_GID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		case AUDIT_SGID:
> @@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct
> audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
>  				break;
>  			default:
> -				return -1;
> +				return -EAU_COMPVALINCOMPAT;
>  			}
>  			break;
>  		default:
> -			return -1;
> +			return -EAU_COMPFIELDINCOMPAT;

This means the same thing.

>  			break;
>  	}
>  	rule->field_count++;
> @@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> **rulep, const char *pair, struct audit_rule_data *rule = *rulep;
> 
>  	if (f == NULL)
> -		return -1;
> +		return -EAU_DATAMISSING;

This also means that the filter was not given. Patch not applied.

Was there a patch in this series that converted errormsg.h to use the macros?

-Steve

>  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
>  		return -EAU_FIELDTOOMANY;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] errormsg: correct a number of messages that have drifted
  2017-05-04 19:50 ` [PATCH 1/2] errormsg: correct a number of messages that have drifted Steve Grubb
@ 2017-05-04 20:25   ` Richard Guy Briggs
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-04 20:25 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-04 15:50, Steve Grubb wrote:
> On Tuesday, April 4, 2017 6:37:47 AM EDT Richard Guy Briggs wrote:
> > A number of error message descriptions have drifted from the conditions that
> > caused them in audit_rule_fieldpair_data() including expansion of fields to
> > be used by the user filter list, restriction to the exit list only and
> > changing an operator to "equals" only.  Correct these, using the new
> > errormsg macros.
> > 
> > See: https://github.com/linux-audit/audit-userspace/issues/12
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> 
> This was hard to check because the first patch switch numbers to macros in 
> libaudit.c, but did not change them in the errormsg tab. It would have been 
> better to apply this first and then change to macros or change everything to 
> macros and then do this last. In any event, it's applied.

Thanks Steve.  I didn't discover the need for this until after I had
done the macro conversion, hence the order of the patches.  I could have
re-ordered the patches to make them easier to check.

> -Steve

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-04 20:11   ` Steve Grubb
@ 2017-05-04 20:29     ` Richard Guy Briggs
  2017-05-04 20:49       ` Steve Grubb
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-04 20:29 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-04 16:11, Steve Grubb wrote:
> On Tuesday, April 4, 2017 6:37:48 AM EDT Richard Guy Briggs wrote:
> > Several return codes were overloaded and no longer giving helpful error
> > return messages from the field and comparison functions
> > audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().
> > 
> > Introduce 3 new macros with more helpful error descriptions for data
> > missing, incompatible fields and incompatible values.
> > 
> > See: https://github.com/linux-audit/audit-userspace/issues/12
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  lib/errormsg.h |    6 ++++++
> >  lib/libaudit.c |   28 ++++++++++++++--------------
> >  2 files changed, 20 insertions(+), 14 deletions(-)
> > 
> > diff --git a/lib/errormsg.h b/lib/errormsg.h
> > index 35b7f95..50c7d50 100644
> > --- a/lib/errormsg.h
> > +++ b/lib/errormsg.h
> > @@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
> >      { -29,    1,    "only takes = operator" },
> >      { -30,    2,    "Field option not supported by kernel:" },
> >      { -31,    1,    "must be used with exclude, user, or exit filter" },
> > +    { -32,    0,    "field data is missing" },
> 
> Actually, this means that the filter is missing in the rule. This is the kind 
> of thing I would normally just fixup after patching the source.
> 
> > +    { -33,    2,    "-C field incompatible" },
> > +    { -34,    2,    "-C value incompatible" },
> >  };
> >  #define EAU_OPMISSING		1
> >  #define EAU_FIELDUNKNOWN	2
> > @@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
> >  #define EAU_OPEQ		29
> >  #define EAU_FIELDNOSUPPORT	30
> >  #define EAU_FIELDNOFILTER	31
> > +#define EAU_DATAMISSING		32
> > +#define EAU_COMPFIELDINCOMPAT	33
> > +#define EAU_COMPVALINCOMPAT	34
> >  #endif
> > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > index b481f52..b1f8f9c 100644
> > --- a/lib/libaudit.c
> > +++ b/lib/libaudit.c
> > @@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, struct audit_rule_data *rule = *rulep;
> > 
> >  	if (f == NULL)
> > -		return -1;
> > +		return -EAU_DATAMISSING;
> > 
> >  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
> >  		return -EAU_FIELDTOOMANY;
> > @@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_EUID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> 
> This means that we are attempting an incompatible comparison between fields.
> 
> >  			}
> >  			break;
> >  		case AUDIT_FSUID:
> > @@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_FSUID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_LOGINUID:
> > @@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_AUID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_SUID:
> > @@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_OBJ_UID:
> > @@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_SUID_TO_OBJ_UID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_UID:
> > @@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> > 
> > @@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_FSGID:
> > @@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_FSGID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_GID:
> > @@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_GID_TO_SGID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_OBJ_GID:
> > @@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_SGID_TO_OBJ_GID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		case AUDIT_SGID:
> > @@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct
> > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> >  				break;
> >  			default:
> > -				return -1;
> > +				return -EAU_COMPVALINCOMPAT;
> >  			}
> >  			break;
> >  		default:
> > -			return -1;
> > +			return -EAU_COMPFIELDINCOMPAT;
> 
> This means the same thing.
> 
> >  			break;
> >  	}
> >  	rule->field_count++;
> > @@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> > **rulep, const char *pair, struct audit_rule_data *rule = *rulep;
> > 
> >  	if (f == NULL)
> > -		return -1;
> > +		return -EAU_DATAMISSING;
> 
> This also means that the filter was not given. Patch not applied.
> 
> Was there a patch in this series that converted errormsg.h to use the macros?

I don't quite follow.  Can you give a fictional example off the top of
your head of what you are hoping for?  I'm hoping to eventually replace
them with an enum list.

> -Steve
> 
> >  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
> >  		return -EAU_FIELDTOOMANY;

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-04 20:29     ` Richard Guy Briggs
@ 2017-05-04 20:49       ` Steve Grubb
  2017-05-04 21:05         ` Richard Guy Briggs
  0 siblings, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2017-05-04 20:49 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: linux-audit

On Thursday, May 4, 2017 4:29:45 PM EDT Richard Guy Briggs wrote:
> On 2017-05-04 16:11, Steve Grubb wrote:
> > On Tuesday, April 4, 2017 6:37:48 AM EDT Richard Guy Briggs wrote:
> > > Several return codes were overloaded and no longer giving helpful error
> > > return messages from the field and comparison functions
> > > audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().
> > > 
> > > Introduce 3 new macros with more helpful error descriptions for data
> > > missing, incompatible fields and incompatible values.
> > > 
> > > See: https://github.com/linux-audit/audit-userspace/issues/12
> > > 
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > > 
> > >  lib/errormsg.h |    6 ++++++
> > >  lib/libaudit.c |   28 ++++++++++++++--------------
> > >  2 files changed, 20 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/lib/errormsg.h b/lib/errormsg.h
> > > index 35b7f95..50c7d50 100644
> > > --- a/lib/errormsg.h
> > > +++ b/lib/errormsg.h
> > > @@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
> > > 
> > >      { -29,    1,    "only takes = operator" },
> > >      { -30,    2,    "Field option not supported by kernel:" },
> > >      { -31,    1,    "must be used with exclude, user, or exit filter"
> > >      },
> > > 
> > > +    { -32,    0,    "field data is missing" },
> > 
> > Actually, this means that the filter is missing in the rule. This is the
> > kind of thing I would normally just fixup after patching the source.
> > 
> > > +    { -33,    2,    "-C field incompatible" },
> > > +    { -34,    2,    "-C value incompatible" },
> > > 
> > >  };
> > >  #define EAU_OPMISSING		1
> > >  #define EAU_FIELDUNKNOWN	2
> > > 
> > > @@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
> > > 
> > >  #define EAU_OPEQ		29
> > >  #define EAU_FIELDNOSUPPORT	30
> > >  #define EAU_FIELDNOFILTER	31
> > > 
> > > +#define EAU_DATAMISSING		32
> > > +#define EAU_COMPFIELDINCOMPAT	33
> > > +#define EAU_COMPVALINCOMPAT	34
> > > 
> > >  #endif
> > > 
> > > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > > index b481f52..b1f8f9c 100644
> > > --- a/lib/libaudit.c
> > > +++ b/lib/libaudit.c
> > > @@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, struct audit_rule_data *rule = *rulep;
> > > 
> > >  	if (f == NULL)
> > > 
> > > -		return -1;
> > > +		return -EAU_DATAMISSING;
> > > 
> > >  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
> > >  	
> > >  		return -EAU_FIELDTOOMANY;
> > > 
> > > @@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_EUID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > 
> > This means that we are attempting an incompatible comparison between
> > fields.> 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_FSUID:
> > > @@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_FSUID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_LOGINUID:
> > > @@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_AUID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_SUID:
> > > @@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_OBJ_UID:
> > > @@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_SUID_TO_OBJ_UID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_UID:
> > > @@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > > 
> > > @@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_FSGID:
> > > @@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_FSGID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_GID:
> > > @@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_GID_TO_SGID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_OBJ_GID:
> > > @@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_SGID_TO_OBJ_GID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		case AUDIT_SGID:
> > > @@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct
> > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > 
> > >  				break;
> > >  			
> > >  			default:
> > > -				return -1;
> > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > >  			}
> > >  			break;
> > >  		
> > >  		default:
> > > -			return -1;
> > > +			return -EAU_COMPFIELDINCOMPAT;
> > 
> > This means the same thing.
> > 
> > >  			break;
> > >  	
> > >  	}
> > >  	rule->field_count++;
> > > 
> > > @@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct
> > > audit_rule_data
> > > **rulep, const char *pair, struct audit_rule_data *rule = *rulep;
> > > 
> > >  	if (f == NULL)
> > > 
> > > -		return -1;
> > > +		return -EAU_DATAMISSING;
> > 
> > This also means that the filter was not given. Patch not applied.
> > 
> > Was there a patch in this series that converted errormsg.h to use the
> > macros?
> I don't quite follow.  Can you give a fictional example off the top of
> your head of what you are hoping for? 

This table:

static const struct msg_tab err_msgtab[] = {
    { -1,    2,    "-F missing operation for" },
    { -2,    2,    "-F unknown field:" },
    { -3,    1,    "must be before -S" },
    { -4,    1,    "machine type not found" },
     ...

converted to using the defines. The libaudit return codes were fixed to 
defines. But the table the return codes are looked up in is still using 
numbers.

> I'm hoping to eventually replace them with an enum list.

define, enum, does it really matter? I don't like lots of patches just 
shuffling things around. Let's just keep it a define at this point.

-Steve

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-04 20:49       ` Steve Grubb
@ 2017-05-04 21:05         ` Richard Guy Briggs
  2017-05-04 21:09           ` Steve Grubb
  2017-05-08 13:48           ` errormsg table macros [was: Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes] Richard Guy Briggs
  0 siblings, 2 replies; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-04 21:05 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-04 16:49, Steve Grubb wrote:
> On Thursday, May 4, 2017 4:29:45 PM EDT Richard Guy Briggs wrote:
> > On 2017-05-04 16:11, Steve Grubb wrote:
> > > On Tuesday, April 4, 2017 6:37:48 AM EDT Richard Guy Briggs wrote:
> > > > Several return codes were overloaded and no longer giving helpful error
> > > > return messages from the field and comparison functions
> > > > audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().
> > > > 
> > > > Introduce 3 new macros with more helpful error descriptions for data
> > > > missing, incompatible fields and incompatible values.
> > > > 
> > > > See: https://github.com/linux-audit/audit-userspace/issues/12
> > > > 
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > ---
> > > > 
> > > >  lib/errormsg.h |    6 ++++++
> > > >  lib/libaudit.c |   28 ++++++++++++++--------------
> > > >  2 files changed, 20 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/lib/errormsg.h b/lib/errormsg.h
> > > > index 35b7f95..50c7d50 100644
> > > > --- a/lib/errormsg.h
> > > > +++ b/lib/errormsg.h
> > > > @@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
> > > > 
> > > >      { -29,    1,    "only takes = operator" },
> > > >      { -30,    2,    "Field option not supported by kernel:" },
> > > >      { -31,    1,    "must be used with exclude, user, or exit filter"
> > > >      },
> > > > 
> > > > +    { -32,    0,    "field data is missing" },
> > > 
> > > Actually, this means that the filter is missing in the rule. This is the
> > > kind of thing I would normally just fixup after patching the source.
> > > 
> > > > +    { -33,    2,    "-C field incompatible" },
> > > > +    { -34,    2,    "-C value incompatible" },
> > > > 
> > > >  };
> > > >  #define EAU_OPMISSING		1
> > > >  #define EAU_FIELDUNKNOWN	2
> > > > 
> > > > @@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
> > > > 
> > > >  #define EAU_OPEQ		29
> > > >  #define EAU_FIELDNOSUPPORT	30
> > > >  #define EAU_FIELDNOFILTER	31
> > > > 
> > > > +#define EAU_DATAMISSING		32
> > > > +#define EAU_COMPFIELDINCOMPAT	33
> > > > +#define EAU_COMPVALINCOMPAT	34
> > > > 
> > > >  #endif
> > > > 
> > > > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > > > index b481f52..b1f8f9c 100644
> > > > --- a/lib/libaudit.c
> > > > +++ b/lib/libaudit.c
> > > > @@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, struct audit_rule_data *rule = *rulep;
> > > > 
> > > >  	if (f == NULL)
> > > > 
> > > > -		return -1;
> > > > +		return -EAU_DATAMISSING;
> > > > 
> > > >  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
> > > >  	
> > > >  		return -EAU_FIELDTOOMANY;
> > > > 
> > > > @@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_EUID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > 
> > > This means that we are attempting an incompatible comparison between
> > > fields.> 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_FSUID:
> > > > @@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_FSUID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_LOGINUID:
> > > > @@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_AUID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_SUID:
> > > > @@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_OBJ_UID:
> > > > @@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_SUID_TO_OBJ_UID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_UID:
> > > > @@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > > 
> > > > @@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_FSGID:
> > > > @@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_FSGID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_GID:
> > > > @@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_GID_TO_SGID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_OBJ_GID:
> > > > @@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_SGID_TO_OBJ_GID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		case AUDIT_SGID:
> > > > @@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct
> > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > > 
> > > >  				break;
> > > >  			
> > > >  			default:
> > > > -				return -1;
> > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > >  			}
> > > >  			break;
> > > >  		
> > > >  		default:
> > > > -			return -1;
> > > > +			return -EAU_COMPFIELDINCOMPAT;
> > > 
> > > This means the same thing.
> > > 
> > > >  			break;
> > > >  	
> > > >  	}
> > > >  	rule->field_count++;
> > > > 
> > > > @@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct
> > > > audit_rule_data
> > > > **rulep, const char *pair, struct audit_rule_data *rule = *rulep;
> > > > 
> > > >  	if (f == NULL)
> > > > 
> > > > -		return -1;
> > > > +		return -EAU_DATAMISSING;
> > > 
> > > This also means that the filter was not given. Patch not applied.

Ok, so coming back to patch acceptance, if I read correctly your
comments, reduce the four new error types to two?

> > > Was there a patch in this series that converted errormsg.h to use the
> > > macros?
> > I don't quite follow.  Can you give a fictional example off the top of
> > your head of what you are hoping for? 
> 
> This table:
> 
> static const struct msg_tab err_msgtab[] = {
>     { -1,    2,    "-F missing operation for" },
>     { -2,    2,    "-F unknown field:" },
>     { -3,    1,    "must be before -S" },
>     { -4,    1,    "machine type not found" },
>      ...
> 
> converted to using the defines. The libaudit return codes were fixed to 
> defines. But the table the return codes are looked up in is still using 
> numbers.

Ah, got it, yes, completely agree.

> > I'm hoping to eventually replace them with an enum list.
> 
> define, enum, does it really matter? I don't like lots of patches just 
> shuffling things around. Let's just keep it a define at this point.

Fair enough.

> -Steve

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-04 21:05         ` Richard Guy Briggs
@ 2017-05-04 21:09           ` Steve Grubb
  2017-05-08 13:52             ` Richard Guy Briggs
  2017-05-08 13:48           ` errormsg table macros [was: Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes] Richard Guy Briggs
  1 sibling, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2017-05-04 21:09 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: linux-audit

On Thursday, May 4, 2017 5:05:35 PM EDT Richard Guy Briggs wrote:
> On 2017-05-04 16:49, Steve Grubb wrote:
> > On Thursday, May 4, 2017 4:29:45 PM EDT Richard Guy Briggs wrote:
> > > On 2017-05-04 16:11, Steve Grubb wrote:
> > > > On Tuesday, April 4, 2017 6:37:48 AM EDT Richard Guy Briggs wrote:
> > > > > Several return codes were overloaded and no longer giving helpful
> > > > > error
> > > > > return messages from the field and comparison functions
> > > > > audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().
> > > > > 
> > > > > Introduce 3 new macros with more helpful error descriptions for data
> > > > > missing, incompatible fields and incompatible values.
> > > > > 
> > > > > See: https://github.com/linux-audit/audit-userspace/issues/12
> > > > > 
> > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > > ---
> > > > > 
> > > > >  lib/errormsg.h |    6 ++++++
> > > > >  lib/libaudit.c |   28 ++++++++++++++--------------
> > > > >  2 files changed, 20 insertions(+), 14 deletions(-)
> > > > > 
> > > > > diff --git a/lib/errormsg.h b/lib/errormsg.h
> > > > > index 35b7f95..50c7d50 100644
> > > > > --- a/lib/errormsg.h
> > > > > +++ b/lib/errormsg.h
> > > > > @@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
> > > > > 
> > > > >      { -29,    1,    "only takes = operator" },
> > > > >      { -30,    2,    "Field option not supported by kernel:" },
> > > > >      { -31,    1,    "must be used with exclude, user, or exit
> > > > >      filter"
> > > > >      },
> > > > > 
> > > > > +    { -32,    0,    "field data is missing" },
> > > > 
> > > > Actually, this means that the filter is missing in the rule. This is
> > > > the
> > > > kind of thing I would normally just fixup after patching the source.
> > > > 
> > > > > +    { -33,    2,    "-C field incompatible" },
> > > > > +    { -34,    2,    "-C value incompatible" },
> > > > > 
> > > > >  };
> > > > >  #define EAU_OPMISSING		1
> > > > >  #define EAU_FIELDUNKNOWN	2
> > > > > 
> > > > > @@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
> > > > > 
> > > > >  #define EAU_OPEQ		29
> > > > >  #define EAU_FIELDNOSUPPORT	30
> > > > >  #define EAU_FIELDNOFILTER	31
> > > > > 
> > > > > +#define EAU_DATAMISSING		32
> > > > > +#define EAU_COMPFIELDINCOMPAT	33
> > > > > +#define EAU_COMPVALINCOMPAT	34
> > > > > 
> > > > >  #endif
> > > > > 
> > > > > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > > > > index b481f52..b1f8f9c 100644
> > > > > --- a/lib/libaudit.c
> > > > > +++ b/lib/libaudit.c
> > > > > @@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, struct audit_rule_data *rule = *rulep;
> > > > > 
> > > > >  	if (f == NULL)
> > > > > 
> > > > > -		return -1;
> > > > > +		return -EAU_DATAMISSING;
> > > > > 
> > > > >  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
> > > > >  	
> > > > >  		return -EAU_FIELDTOOMANY;
> > > > > 
> > > > > @@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_EUID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > 
> > > > This means that we are attempting an incompatible comparison between
> > > > fields.>
> > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_FSUID:
> > > > > @@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_FSUID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_LOGINUID:
> > > > > @@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_AUID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_SUID:
> > > > > @@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_OBJ_UID:
> > > > > @@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_SUID_TO_OBJ_UID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_UID:
> > > > > @@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > > 
> > > > > @@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_FSGID:
> > > > > @@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_FSGID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_GID:
> > > > > @@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_GID_TO_SGID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_OBJ_GID:
> > > > > @@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_SGID_TO_OBJ_GID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		case AUDIT_SGID:
> > > > > @@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct
> > > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > > > 
> > > > >  				break;
> > > > >  			
> > > > >  			default:
> > > > > -				return -1;
> > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > >  			}
> > > > >  			break;
> > > > >  		
> > > > >  		default:
> > > > > -			return -1;
> > > > > +			return -EAU_COMPFIELDINCOMPAT;
> > > > 
> > > > This means the same thing.
> > > > 
> > > > >  			break;
> > > > >  	
> > > > >  	}
> > > > >  	rule->field_count++;
> > > > > 
> > > > > @@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct
> > > > > audit_rule_data
> > > > > **rulep, const char *pair, struct audit_rule_data *rule = *rulep;
> > > > > 
> > > > >  	if (f == NULL)
> > > > > 
> > > > > -		return -1;
> > > > > +		return -EAU_DATAMISSING;
> > > > 
> > > > This also means that the filter was not given. Patch not applied.
> 
> Ok, so coming back to patch acceptance, if I read correctly your
> comments, reduce the four new error types to two?

Yes, two are needed. One for missing filter/action and one for we are 
attempting an incompatible comparison between fields.

-Steve

> > > > Was there a patch in this series that converted errormsg.h to use the
> > > > macros?
> > > 
> > > I don't quite follow.  Can you give a fictional example off the top of
> > > your head of what you are hoping for?
> > 
> > This table:
> > 
> > static const struct msg_tab err_msgtab[] = {
> > 
> >     { -1,    2,    "-F missing operation for" },
> >     { -2,    2,    "-F unknown field:" },
> >     { -3,    1,    "must be before -S" },
> >     { -4,    1,    "machine type not found" },
> >     
> >      ...
> > 
> > converted to using the defines. The libaudit return codes were fixed to
> > defines. But the table the return codes are looked up in is still using
> > numbers.
> 
> Ah, got it, yes, completely agree.
> 
> > > I'm hoping to eventually replace them with an enum list.
> > 
> > define, enum, does it really matter? I don't like lots of patches just
> > shuffling things around. Let's just keep it a define at this point.
> 
> Fair enough.
> 
> > -Steve
> 
> - RGB
> 
> --
> Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* errormsg table macros [was: Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes]
  2017-05-04 21:05         ` Richard Guy Briggs
  2017-05-04 21:09           ` Steve Grubb
@ 2017-05-08 13:48           ` Richard Guy Briggs
  1 sibling, 0 replies; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-08 13:48 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-04 17:05, Richard Guy Briggs wrote:
> On 2017-05-04 16:49, Steve Grubb wrote:
> > On Thursday, May 4, 2017 4:29:45 PM EDT Richard Guy Briggs wrote:
> > > On 2017-05-04 16:11, Steve Grubb wrote:
> > > > Was there a patch in this series that converted errormsg.h to use the
> > > > macros?
> > > I don't quite follow.  Can you give a fictional example off the top of
> > > your head of what you are hoping for? 
> > 
> > This table:
> > 
> > static const struct msg_tab err_msgtab[] = {
> >     { -1,    2,    "-F missing operation for" },
> >     { -2,    2,    "-F unknown field:" },
> >     { -3,    1,    "must be before -S" },
> >     { -4,    1,    "machine type not found" },
> >      ...
> > 
> > converted to using the defines. The libaudit return codes were fixed to 
> > defines. But the table the return codes are looked up in is still using 
> > numbers.
> 
> Ah, got it, yes, completely agree.

Ok, here is a pull request to make that change:
	https://github.com/linux-audit/audit-userspace/pull/22

I've tagged it related to: https://github.com/linux-audit/audit-userspace/issues/11

> > -Steve
> 
> - RGB

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-04 21:09           ` Steve Grubb
@ 2017-05-08 13:52             ` Richard Guy Briggs
  2017-05-24 20:02               ` Steve Grubb
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-08 13:52 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-04 17:09, Steve Grubb wrote:
> On Thursday, May 4, 2017 5:05:35 PM EDT Richard Guy Briggs wrote:
> > On 2017-05-04 16:49, Steve Grubb wrote:
> > > On Thursday, May 4, 2017 4:29:45 PM EDT Richard Guy Briggs wrote:
> > > > On 2017-05-04 16:11, Steve Grubb wrote:
> > > > > On Tuesday, April 4, 2017 6:37:48 AM EDT Richard Guy Briggs wrote:
> > > > > > Several return codes were overloaded and no longer giving helpful
> > > > > > error
> > > > > > return messages from the field and comparison functions
> > > > > > audit_rule_fieldpair_data() and audit_rule_interfield_comp_data().
> > > > > > 
> > > > > > Introduce 3 new macros with more helpful error descriptions for data
> > > > > > missing, incompatible fields and incompatible values.
> > > > > > 
> > > > > > See: https://github.com/linux-audit/audit-userspace/issues/12
> > > > > > 
> > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > > > ---
> > > > > > 
> > > > > >  lib/errormsg.h |    6 ++++++
> > > > > >  lib/libaudit.c |   28 ++++++++++++++--------------
> > > > > >  2 files changed, 20 insertions(+), 14 deletions(-)
> > > > > > 
> > > > > > diff --git a/lib/errormsg.h b/lib/errormsg.h
> > > > > > index 35b7f95..50c7d50 100644
> > > > > > --- a/lib/errormsg.h
> > > > > > +++ b/lib/errormsg.h
> > > > > > @@ -67,6 +67,9 @@ static const struct msg_tab err_msgtab[] = {
> > > > > > 
> > > > > >      { -29,    1,    "only takes = operator" },
> > > > > >      { -30,    2,    "Field option not supported by kernel:" },
> > > > > >      { -31,    1,    "must be used with exclude, user, or exit
> > > > > >      filter"
> > > > > >      },
> > > > > > 
> > > > > > +    { -32,    0,    "field data is missing" },
> > > > > 
> > > > > Actually, this means that the filter is missing in the rule. This is
> > > > > the
> > > > > kind of thing I would normally just fixup after patching the source.
> > > > > 
> > > > > > +    { -33,    2,    "-C field incompatible" },
> > > > > > +    { -34,    2,    "-C value incompatible" },
> > > > > > 
> > > > > >  };
> > > > > >  #define EAU_OPMISSING		1
> > > > > >  #define EAU_FIELDUNKNOWN	2
> > > > > > 
> > > > > > @@ -97,4 +100,7 @@ static const struct msg_tab err_msgtab[] = {
> > > > > > 
> > > > > >  #define EAU_OPEQ		29
> > > > > >  #define EAU_FIELDNOSUPPORT	30
> > > > > >  #define EAU_FIELDNOFILTER	31
> > > > > > 
> > > > > > +#define EAU_DATAMISSING		32
> > > > > > +#define EAU_COMPFIELDINCOMPAT	33
> > > > > > +#define EAU_COMPVALINCOMPAT	34
> > > > > > 
> > > > > >  #endif
> > > > > > 
> > > > > > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > > > > > index b481f52..b1f8f9c 100644
> > > > > > --- a/lib/libaudit.c
> > > > > > +++ b/lib/libaudit.c
> > > > > > @@ -976,7 +976,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, struct audit_rule_data *rule = *rulep;
> > > > > > 
> > > > > >  	if (f == NULL)
> > > > > > 
> > > > > > -		return -1;
> > > > > > +		return -EAU_DATAMISSING;
> > > > > > 
> > > > > >  	if (rule->field_count >= (AUDIT_MAX_FIELDS - 1))
> > > > > >  	
> > > > > >  		return -EAU_FIELDTOOMANY;
> > > > > > 
> > > > > > @@ -1043,7 +1043,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_EUID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > 
> > > > > This means that we are attempting an incompatible comparison between
> > > > > fields.>
> > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_FSUID:
> > > > > > @@ -1069,7 +1069,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_FSUID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_LOGINUID:
> > > > > > @@ -1095,7 +1095,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_AUID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_SUID:
> > > > > > @@ -1121,7 +1121,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_OBJ_UID:
> > > > > > @@ -1147,7 +1147,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_SUID_TO_OBJ_UID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_UID:
> > > > > > @@ -1173,7 +1173,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_UID_TO_SUID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > > 
> > > > > > @@ -1197,7 +1197,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_FSGID:
> > > > > > @@ -1219,7 +1219,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_FSGID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_GID:
> > > > > > @@ -1241,7 +1241,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_GID_TO_SGID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_OBJ_GID:
> > > > > > @@ -1263,7 +1263,7 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_SGID_TO_OBJ_GID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		case AUDIT_SGID:
> > > > > > @@ -1285,11 +1285,11 @@ int audit_rule_interfield_comp_data(struct
> > > > > > audit_rule_data **rulep, AUDIT_COMPARE_EGID_TO_SGID;
> > > > > > 
> > > > > >  				break;
> > > > > >  			
> > > > > >  			default:
> > > > > > -				return -1;
> > > > > > +				return -EAU_COMPVALINCOMPAT;
> > > > > > 
> > > > > >  			}
> > > > > >  			break;
> > > > > >  		
> > > > > >  		default:
> > > > > > -			return -1;
> > > > > > +			return -EAU_COMPFIELDINCOMPAT;
> > > > > 
> > > > > This means the same thing.
> > > > > 
> > > > > >  			break;
> > > > > >  	
> > > > > >  	}
> > > > > >  	rule->field_count++;
> > > > > > 
> > > > > > @@ -1389,7 +1389,7 @@ int audit_rule_fieldpair_data(struct
> > > > > > audit_rule_data
> > > > > > **rulep, const char *pair, struct audit_rule_data *rule = *rulep;
> > > > > > 
> > > > > >  	if (f == NULL)
> > > > > > 
> > > > > > -		return -1;
> > > > > > +		return -EAU_DATAMISSING;
> > > > > 
> > > > > This also means that the filter was not given. Patch not applied.
> > 
> > Ok, so coming back to patch acceptance, if I read correctly your
> > comments, reduce the four new error types to two?
> 
> Yes, two are needed. One for missing filter/action and one for we are 
> attempting an incompatible comparison between fields.

Ok, here you go:
	https://github.com/linux-audit/audit-userspace/pull/21

> -Steve
> 
> > > > > Was there a patch in this series that converted errormsg.h to use the
> > > > > macros?
> > > > 
> > > > I don't quite follow.  Can you give a fictional example off the top of
> > > > your head of what you are hoping for?
> > > 
> > > This table:
> > > 
> > > static const struct msg_tab err_msgtab[] = {
> > > 
> > >     { -1,    2,    "-F missing operation for" },
> > >     { -2,    2,    "-F unknown field:" },
> > >     { -3,    1,    "must be before -S" },
> > >     { -4,    1,    "machine type not found" },
> > >     
> > >      ...
> > > 
> > > converted to using the defines. The libaudit return codes were fixed to
> > > defines. But the table the return codes are looked up in is still using
> > > numbers.
> > 
> > Ah, got it, yes, completely agree.
> > 
> > > > I'm hoping to eventually replace them with an enum list.
> > > 
> > > define, enum, does it really matter? I don't like lots of patches just
> > > shuffling things around. Let's just keep it a define at this point.
> > 
> > Fair enough.
> > 
> > > -Steve
> > 
> > - RGB

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-08 13:52             ` Richard Guy Briggs
@ 2017-05-24 20:02               ` Steve Grubb
  2017-05-24 21:46                 ` Richard Guy Briggs
  0 siblings, 1 reply; 14+ messages in thread
From: Steve Grubb @ 2017-05-24 20:02 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: linux-audit

On Monday, May 8, 2017 9:52:00 AM EDT Richard Guy Briggs wrote:
> > > Ok, so coming back to patch acceptance, if I read correctly your
> > > comments, reduce the four new error types to two?
> > 
> > Yes, two are needed. One for missing filter/action and one for we are
> > attempting an incompatible comparison between fields.
> 
> Ok, here you go:
>         https://github.com/linux-audit/audit-userspace/pull/21

Was just going to look over this patch and now I see its not on the mail list. 
This version looks fine. I kind of prefer patches sent to the mail list so 
that any discussion is archived. 

-Steve

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-24 20:02               ` Steve Grubb
@ 2017-05-24 21:46                 ` Richard Guy Briggs
  2017-05-29 15:36                   ` Richard Guy Briggs
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-24 21:46 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-24 16:02, Steve Grubb wrote:
> On Monday, May 8, 2017 9:52:00 AM EDT Richard Guy Briggs wrote:
> > > > Ok, so coming back to patch acceptance, if I read correctly your
> > > > comments, reduce the four new error types to two?
> > > 
> > > Yes, two are needed. One for missing filter/action and one for we are
> > > attempting an incompatible comparison between fields.
> > 
> > Ok, here you go:
> >         https://github.com/linux-audit/audit-userspace/pull/21
> 
> Was just going to look over this patch and now I see its not on the mail list. 
> This version looks fine. I kind of prefer patches sent to the mail list so 
> that any discussion is archived. 

No problem.  I'll continue sending patches to the list.  Now that the
repo is on github, it is so much more convenient to just push the
changes on one of my branches and make a pull request.  I expect it is
equally easy to accept a pull request and merge straight on github.

> -Steve

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes
  2017-05-24 21:46                 ` Richard Guy Briggs
@ 2017-05-29 15:36                   ` Richard Guy Briggs
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Guy Briggs @ 2017-05-29 15:36 UTC (permalink / raw)
  To: Steve Grubb; +Cc: linux-audit

On 2017-05-24 17:46, Richard Guy Briggs wrote:
> On 2017-05-24 16:02, Steve Grubb wrote:
> > On Monday, May 8, 2017 9:52:00 AM EDT Richard Guy Briggs wrote:
> > > > > Ok, so coming back to patch acceptance, if I read correctly your
> > > > > comments, reduce the four new error types to two?
> > > > 
> > > > Yes, two are needed. One for missing filter/action and one for we are
> > > > attempting an incompatible comparison between fields.
> > > 
> > > Ok, here you go:
> > >         https://github.com/linux-audit/audit-userspace/pull/21
> > 
> > Was just going to look over this patch and now I see its not on the mail list. 
> > This version looks fine. I kind of prefer patches sent to the mail list so 
> > that any discussion is archived. 
> 
> No problem.  I'll continue sending patches to the list.  Now that the
> repo is on github, it is so much more convenient to just push the
> changes on one of my branches and make a pull request.  I expect it is
> equally easy to accept a pull request and merge straight on github.

Ok, I hadn't understood that you hadn't applied it despite reviewing it
and finding it fine.  I'll send it to the list now.

> > -Steve
> 
> - RGB

- RGB

--
Richard Guy Briggs <rgb@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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-05-29 15:36 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-04 10:37 [PATCH 1/2] errormsg: correct a number of messages that have drifted Richard Guy Briggs
2017-04-04 10:37 ` [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes Richard Guy Briggs
2017-05-04 20:11   ` Steve Grubb
2017-05-04 20:29     ` Richard Guy Briggs
2017-05-04 20:49       ` Steve Grubb
2017-05-04 21:05         ` Richard Guy Briggs
2017-05-04 21:09           ` Steve Grubb
2017-05-08 13:52             ` Richard Guy Briggs
2017-05-24 20:02               ` Steve Grubb
2017-05-24 21:46                 ` Richard Guy Briggs
2017-05-29 15:36                   ` Richard Guy Briggs
2017-05-08 13:48           ` errormsg table macros [was: Re: [PATCH 2/2] errormsg: add descriptive macros to replace overloaded error codes] Richard Guy Briggs
2017-05-04 19:50 ` [PATCH 1/2] errormsg: correct a number of messages that have drifted Steve Grubb
2017-05-04 20:25   ` Richard Guy Briggs

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.