[apparmor] [patch 02/24] Convert mount and dbus to be subclasses of a generic rule class

John Johansen john.johansen at canonical.com
Tue Mar 11 07:42:25 UTC 2014


On 03/10/2014 11:40 PM, Seth Arnold wrote:
> On Fri, Mar 07, 2014 at 09:31:23AM -0800, john.johansen at canonical.com wrote:
>> This will simplify add new features as most of the code can reside in
>> its own class. There are still things to improve but its a start.
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
> 
> Sorry, I only made it through the first half of the patch. Some small
> notes inline:
> 

<< snip >>
 
>> -	DUP_STRING(orig, entry, mnt_point, err);
>> -	DUP_STRING(orig, entry, device, err);
>> -	DUP_STRING(orig, entry, trans, err);
>> +	if (flags == MS_ALL_FLAGS) {
>> +		/* all flags are optional */
>> +	  len = snprintf(p, size, "%s", default_match_pattern);
>> +		if (len < 0 || len >= size)
>> +			return FALSE;
>> +		return TRUE;
>> +	}
> 
> Some odd whitespace with the line starting len = snprintf
> 
thanks

<< snip >>

>> +	if (!count)
>> +		/* didn't actually encode anything */
>> +		goto fail;
>> +
>> +	prof.policy.count++;
>> +	return RULE_OK;
>> +
> 
> Is prof.policy.count++ correct or should that be:
> prof.policy.count += count; ?
> 
debatable, are you counting the number of aare rules added to the policy,
or the number of backend pcre rules add. With how it is currently used
it doesn't matter.

Also this part gets changed in a patch further in where the counting gets
moved into the rules class backend (so we start counting pcre rules added).


Also note, the abstracting in these patches aren't as far along as I would
like but its a start.




More information about the AppArmor mailing list