On 08/20/2014 11:45 PM, Seth Arnold wrote:
> On Fri, Aug 15, 2014 at 12:20:42PM -0700, [email protected] wrote:
>> We need to rework permission type mapping to nodesets, which means we
>> need to move the nodeset computations earlier in the dfa creation
>> processes, instead of a post step of follow(), so move the nodeset
>> into expr-tree
>>
>> Signed-off-by: John Johansen <[email protected]>
>
> I asked some questions inline, but since this patch didn't introduce any
> of what I'm curious about:
>
> Acked-by: Seth Arnold <[email protected]>
>
<<snip>>
>> + NodeSet *insert(NodeSet *nodes)
>> + {
>> + if (!nodes)
>> + return NULL;
>> + pair<set<hashedNodeSet>::iterator,bool> uniq;
>> + uniq = cache.insert(hashedNodeSet(nodes));
>
> Does this make two hashedNodeSet objects, one on the stack and then one
> in the 'cache' set<>? Or does this copy stack object into the set<>? Would
> a shared_ptr or new here only create one?
>
Welcome to the hell of C++ with its copy, assignment and reference constructors
etc.
The first line declares an object that may or may not be built with the null
constructor at the discretion of the compiler, the second assigns it. The
compiler should notice this and not create the first object but, it could
just destroy it instead.
>> + if (uniq.second == false) {
>> + delete(nodes);
>> + dup++;
>
> Is it expected behaviour for set::insert to delete its argument?
>
in this case yes, but its not really something that is a standard practice.
This is some of the code that needs to go away. We need to be properly
accumulating
these from the beginning as separate sets, instead of resplitting them after
the fact.
>> + } else {
>> + sum += nodes->size();
>> + if (nodes->size() > max)
>> + max = nodes->size();
>> + }
>> + return uniq.first->nodes;
>> + }
>> +};
>> +
>> +struct deref_less_than {
>> + bool operator()(hashedNodeVec * const &lhs, hashedNodeVec * const
>> &rhs)const
>> + {
>> + return *lhs < *rhs;
>> + }
>> +};
>
> There's funny whitespace all over this little class -- the 'const' at the
> end of the prototype is missing a space and I'm pretty sure the braces are
> in the wrong column.
>
feel free to provide a ws patch
--
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
