On 02/15/2017 08:46 PM, Seth Arnold wrote:
> This patch was mostly good with a few questions:
> 
> Also, I noticed all the copyright years need to be updated.
> 
> On Fri, Feb 10, 2017 at 12:51:49PM -0800, John Johansen wrote:
>> +    info->rpaths = malloc(info->rn * sizeof(*info->rpaths));
>> +    info->rwpaths = malloc(info->rwn * sizeof(*info->rwpaths));
>> +    info->arpaths = malloc(info->arn * sizeof(*info->arpaths));
>> +    info->arwpaths = malloc(info->arwn * sizeof(*info->arwpaths));
> 
> I'd rather see these as calloc() or similar overflow-gaurding methods.
> 
>> +std::ostream &dconf_rule::dump(std::ostream &os)
>> +{
>> +    if (audit)
>> +            os << "audit ";
>> +
>> +    os << "dconf (";
>> +
>> +    switch (mode & AA_DCONF_READWRITE) {
>> +    case AA_DCONF_READ:
>> +            os << "read";
>> +            break;
>> +    case AA_DCONF_WRITE:
>> +            os << " write";
>> +            break;
>> +    }
>> +    os << ") " << path << ",\n";
>> +
>> +    return os;
>> +}
> 
> I think it's unspecified what will happen if both read and write are
> selected here; I'm pretty sure the grammar allows both to be enabled
> at once.
> 
The grammar has a semantic check in the yacc portion requiring either
r or rw  but not w

> 
>> +class DataEnt {
>> +public:
>> +    virtual ~DataEnt() = 0;
>> +    virtual void serialize(std::ostringstream &buf) = 0;
>> +};
>> +/* just in case the base case destructor ever gets invoked */
>> +inline DataEnt::~DataEnt() { };
> 
> Is it kosher to set it to 0 above but then define it here?
> 
it won't break things but yeah should fix.


-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to