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

[Seth Arnold]

On Mon, Mar 14, 2016 at 10:39:09PM -0500, Tyler Hicks wrote:
> -		yyerror(_("Unknown error while parsing label: %s\n"), label);

> +			yyerror(_(err), label);

Does this change break the translator tools that gather strings to
translate? (It's a nice cleanup otherwise.)

>  	if (!*_name) {
>  		free(*_ns);
> -		yyerror(_("Memory allocation error."));
> +		goto alloc_fail;
>  	}
> +
> +	return true;
> +
> +alloc_fail:
> +	err = "Memory allocation error.";
> +	if (yyerr)
> +		yyerror(_(err));
> +	else
> +		fprintf(stderr, "%s", _(err));
> +
> +	return false;

This appears to be a behaviour change beyond a cleanup -- previously it'd
call yyerror() then return true. I suspect this is intentional but I
wanted to call it out as more than a cleanup...

> +		if (!kernel_supports_stacking) {
> +			bool stack;
> +
> +			if (!parse_label(&stack, &ns, &name,
> +					 tbuf.c_str(), false)) {
> +				return FALSE;
> +			} else if (stack) {

I'm not a fan of this construct:

if (set_variable(&foo))
  /* branch A */
else if (foo)
  /* branch B */

This actually makes branch B dependant upon both the return value of the
function _and_ the variable but that's not obvious as the two are
separated by some distance. (It's not much, here, but it's enough to be
confusing.)

There's four cases of return-value and stack and it's not clear on first
glance how many of them are handled explicitly.

> +					_("The current kernel does not support stacking of named transitions: %s\n"),
> +					tbuf.c_str());
> +				return FALSE;
> +			}


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/20160314/6637ffa9/attachment.pgp>