[apparmor] [Patch] abstract out the directory walking routine

[Steve Beattie]

On Wed, Aug 15, 2012 at 02:50:27AM -0700, John Johansen wrote:
> apparmor: abstract out the directory walking routine
> 
> The apparmor_parser has 3 different directory walking routines. Abstract
> them out and use a single common routine.
>
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  parser/Makefile       |    7 +-
>  parser/lib.c          |  117 +++++++++++++++++++++++++++++

lib.c needs a copyright header. It can also probably merged with
parser_misc.c as they're kind of intended to serve the same purpose;
helper functions that don't fit in a specific location, but that's not
a blocker for this patch.

>  parser/lib.h          |    7 ++
>  parser/parser_lex.l   |  111 +++++++++++++++-------------
>  parser/parser_main.c  |  197 ++++++++++++++-----------------------------------
>  parser/tst/caching.sh |    2 +-
>  6 files changed, 248 insertions(+), 193 deletions(-)
>  create mode 100644 parser/lib.c
>  create mode 100644 parser/lib.h
> 
> diff --git a/parser/Makefile b/parser/Makefile
> index 0a31226..1d3dc6a 100644
> diff --git a/parser/lib.c b/parser/lib.c
> new file mode 100644
> index 0000000..7c69c68
> --- /dev/null
> +++ b/parser/lib.c
> @@ -0,0 +1,117 @@
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stddef.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#define _(s) gettext(s)
> +
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +
> +#include "parser.h"
> +
> +/**
> + * dirat_for_each: iterate over a directory calling cb for each entry
> + * @dir: already opened directory (MAYBE NULL)
> + * @name: name of the directory (MAYBE NULL)

grammar nit "MAY BE NULL"

I do wonder if it wouldn't be better to have three versions of this
function, one as-is requiring @dir and @name to be non-NULL, and the
other two for the single argument version of the functions (I suspect
the one with @dir only would be called by the other two, but I haven't
thought very hard about it). This would potentially be a little more
defensive programming as a caller that meant to pass a non-NULL as one
of the arguments but messed up for some reason would generate an error,
and not be accepted with unexpected behavior.

Otherwise, code looks fine, and none of my objections except the
copyright bit should prevent it from going in as-is.

Thanks!

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20120816/3a499a4e/attachment.pgp>