[apparmor] [PATCH 02/12] libapparmor: Move over the lib functions needed by cache loading code

[Seth Arnold]

On Wed, Dec 10, 2014 at 04:12:23PM -0600, Tyler Hicks wrote:
> The function names must be prepended with "_aa_" since they're going to
> be exported from libapparmor. The code bases using the _aa_autofree(),
> _aa_autoclose(), and _aa_autofclose() will need to internally alias
> those functions to the previously used autofree, autoclose, and
> autofclose names.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>

Acked-by: Seth Arnold <seth.arnold at canonical.com>

I think I found an error in the _aa_dirat_for_each() return value:

> +
> +/**
> + * dirat_for_each: iterate over a directory calling cb for each entry

Should be _aa_dirat_for_each

> + * @dir: already opened directory (MAY BE NULL)
> + * @name: name of the directory (MAY BE NULL)
> + * @data: data pointer to pass to the callback fn (MAY BE NULL)
> + * @cb: the callback to pass entry too (NOT NULL)
> + *
> + * Iterate over the entries in a directory calling cb for each entry.
> + * The directory to iterate is determined by a combination of @dir and
> + * @name.
> + *
> + * IF @name is a relative path it is determine relative to at @dir if it
> + * is specified, else it the lookup is done relative to the current
> + * working directory.
> + *
> + * If @name is not specified then @dir is used as the directory to iterate
> + * over.
> + *
> + * It is an error if both @name and @dir are null
> + *
> + * The cb function is called with the DIR in use and the name of the
> + * file in that directory.  If the file is to be opened it should
> + * use the openat, fstatat, and related fns.
> + *
> + * Returns: 0 on success, else -1 and errno is set to the error code

Note -1 and errno set on failure...

> + */
> +int _aa_dirat_for_each(DIR *dir, const char *name, void *data,
> +		       int (* cb)(DIR *, const char *, struct stat *, void *))
> +{

> +     /* .... */

> +
> +	for (error = readdir_r(d, dirent, &ent);     /* XXX */
> +	     error == 0 && ent != NULL;
> +	     error = readdir_r(d, dirent, &ent)) {   /* XXX */
> +		struct stat my_stat;
> +
> +		if (strcmp(ent->d_name, ".") == 0 ||
> +		    strcmp(ent->d_name, "..") == 0)
> +			continue;
> +
> +		if (fstatat(dirfd(d), ent->d_name, &my_stat, 0)) {
> +			goto fail;
> +		}
> +
> +		if (cb(d, ent->d_name, &my_stat, data)) {
> +			goto fail;
> +		}
> +	}
> +
> +	if (d != dir)
> +		closedir(d);
> +
> +	return error;

If either readdir_r() marked with an /* XXX */ fails, they'll return a
positive number as the return value, which is then returned via "return
error". That's not the -1 and errno that the comment suggested.

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150213/36727c0d/attachment.pgp>