On 2015-02-13 17:46:26, Seth Arnold wrote: > 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 <[email protected]> > > Acked-by: Seth Arnold <[email protected]>
Thanks!
>
> 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
Fixed in my local branch. It'll be in the eventual v2 that I send out.
>
> > + * @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.
Nice catch!
I think this is something that should be fixed outside of this patch
set so I've sent a fix to the list, independent of this thread:
https://lists.ubuntu.com/archives/apparmor/2015-February/007229.html
Tyler
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
