[apparmor] [PATCH 07/10] Lindent + some hand cleanups expr-tree

[Seth Arnold]

Thanks John, a few comments inline:

On Thu, Mar 10, 2011 at 12:35 PM, John Johansen
<john.johansen at canonical.com> wrote:
> @@ -249,7 +246,6 @@ static Node *merge_charset(Node *a, Node *b)
>                        to->insert(*i);
>                return b;
>        }
> -
>        //return ???;
>  }

It really would be nice to know more about this. :)

>  typedef unsigned char uchar;
> -typedef set<uchar> Chars;
> +typedef set <uchar> Chars;

This behavior is strange. I'm much more used to the set<uchar> style.

> -ostream& operator<<(ostream& os, uchar c);
> +ostream & operator<<(ostream &os, uchar c);
>
>  /* Compute the union of two sets. */
> -template<class T>
> -set<T> operator+(const set<T>& a, const set<T>& b)
> +template <class T> set <T> operator+(const set <T> &a, const set <T> &b)
>  {
> -       set<T> c(a);
> +       set < T > c(a);

.. and here it is an utter failure. :) Is there a different Lindent mode
for handling C++ templates?

> @@ -83,27 +82,33 @@ ostream& operator<<(ostream& os, const NodeSet& state);
>  * enumerating all the explicit tranitions for default matches.
>  */
>  typedef struct NodeCases {
> -       typedef map<uchar, NodeSet *>::iterator iterator;
> +       typedef map <uchar, NodeSet *>::iterator iterator;

Similar problem here.

>        iterator begin() { return cases.begin(); }
>        iterator end() { return cases.end(); }
>
> -       NodeCases() : otherwise(0) { }
> -       map<uchar, NodeSet *> cases;
> +       NodeCases(): otherwise(0) { }
> +       map <uchar, NodeSet *> cases;

And here. :)

> @@ -340,10 +337,10 @@ public:
>  };
>
>  /* Match any character (/./). */
> -class AnyCharNode : public CNode {
> -public:
> +class AnyCharNode:public CNode {
> +      public:
>        AnyCharNode() { }
> -       void follow(NodeCases& cases)
> +       void follow(NodeCases & cases)

Hrm, elsewhere the ampersand was placed next to the parameter, which I
prefer; the spaces on both sides are strange. :)

> @@ -385,40 +380,32 @@ public:
>        /* requires accept nodes to be common by pointer */
>        int eq(Node *other)
>        {
> -               if (dynamic_cast<AcceptNode *>(other))
> +               if (dynamic_cast < AcceptNode * >(other))

The other instances of replacing
    dynamic_cast<foo *>
with
    dynamic_cast <foo *>
didn't bother me much, but this looks too funny.

> @@ -427,36 +414,26 @@ public:
...
> +class PlusNode:public OneChildNode {
> +      public:
> +       PlusNode(Node *left): OneChildNode(left) {
>        }

Hunh, I'm surprised this is on two lines. It'd be more obviously empty
if both brackets were on the same line.


Thanks!