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

John Johansen john.johansen at canonical.com
Thu Aug 16 23:23:04 UTC 2012


On 08/16/2012 04:02 PM, Steve Beattie wrote:
> On Wed, Aug 15, 2012 at 02:50:27AM -0700, John Johansen wrote:

>>  parser/Makefile       |    7 +-
>>  parser/lib.c          |  117 +++++++++++++++++++++++++++++
> 
> lib.c needs a copyright header. It can also probably merged with
doh right

> 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.
> 
actually my plan was to move things the other way as they got cleaned up.

>>  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"
> 
heh, yeah

> 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.
> 
Hrmmm I'll tinker with it, I am thinking that the others would basically
be wrapper fns.

> Otherwise, code looks fine, and none of my objections except the
> copyright bit should prevent it from going in as-is.
> 
> Thanks!
> 
> 
Okay I'll count that as an acked-by: and send out the tinkering patch
on top of this.




More information about the AppArmor mailing list