[apparmor] [PATCH 4/6] parser: stop splitting the namespace from the named transition targets

[Seth Arnold]

On Tue, Mar 15, 2016 at 10:38:25PM -0500, Tyler Hicks wrote:
> > Do you prefer this instead?
> > 
> > if (set_variable(&foo))
> >   /* branch A */
> > 
> > if (foo)
> >   /* branch B */

I do; it's not identical in the general case but it is in this case and
it's a lot easier for me to read. Thanks.

> -void parse_label(bool *_stack, char **_ns, char **_name, const char *label)
> +bool parse_label(bool *_stack, char **_ns, char **_name,
> +		 const char *label, bool yyerr)
>  {

> +	if (err) {
> +		if (yyerr)
> +			yyerror(err, label);
> +		else
> +			fprintf(stderr, err, label);
> +
> +		return false;
>  	}

Just a passing fancy that if instead of "bool yyerr" the parameter were
"(void)(yyerr(char*, char*))" (or something along those lines) then we could
supply function pointers to describe how to handle the error, and remove
half the code in this block, which we'll probably wind up duplicating into
more and more functions.

Then callers would call e.g.:

parse_label(&stack, &ns, &name, label, &yyerror);

or

void errors_to_stderr(const char *err, const char *label)
{
	fprintf(stderr, err, label);
}
parse_label(&stack, &ns, &name, label, &errors_to_stderr);

Maybe tonight's not the night to sort out our error handling but including
blocks like this everywhere would probably get tiring.

> @@ -585,7 +587,27 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
>  			/* allow change_profile for all execs */
>  			vec[0] = "/[^/\\x00][^\\x00]*";
>  
> -		vec[index++] = tbuf.c_str();
> +		if (!kernel_supports_stacking) {
> +			bool stack;
> +
> +			if (!parse_label(&stack, &ns, &name,
> +					 tbuf.c_str(), false)) {
> +				return FALSE;
> +			}
> +
> +			if (stack) {
> +				fprintf(stderr,
> +					_("The current kernel does not support stacking of named transitions: %s\n"),
> +					tbuf.c_str());
> +				return FALSE;
> +			}

Thanks, this is easier for me to follow.

Another aside, there's a similar error message for audit messages in this
function that's not labeled for translation. We may want to set aside some
time to make a pass through the parser and add or remove messages from
translations.

Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160315/298cbe8e/attachment.pgp>