On 2015-04-01 17:43:53, Seth Arnold wrote: > On Thu, Mar 26, 2015 at 04:47:58PM -0500, Tyler Hicks wrote: > > Two different implementations were in use for reading features files. > > One for reading a single file and another for reading a single file > > after walking a directory. This patch creates a single function that is > > used in both cases. > > > > Signed-off-by: Tyler Hicks <[email protected]> > > Include the same 'dirfd' complaint from earlier. with the same either > ignore it because openat() ignores it rationale, or change the name > because it might collide against dirent.h.
I wonder if the collision is a good thing. It'll make the next person think twice before mixing directory streams and directory file descriptors in the same section of code. Tyler > > Acked-by: Seth Arnold <[email protected]> > > Thanks > > > --- > > libraries/libapparmor/src/features.c | 114 > > +++++++++++++++++++++-------------- > > 1 file changed, 68 insertions(+), 46 deletions(-) > > > > diff --git a/libraries/libapparmor/src/features.c > > b/libraries/libapparmor/src/features.c > > index e7bf745..f913e91 100644 > > --- a/libraries/libapparmor/src/features.c > > +++ b/libraries/libapparmor/src/features.c > > @@ -80,6 +80,69 @@ static int features_snprintf(struct features_struct > > *fst, const char *fmt, ...) > > return 0; > > } > > > > +/* load_features_file - opens and reads a file into @buffer and then > > NUL-terminates @buffer > > + * @dirfd: a directory file descriptory or AT_FDCWD (see openat(2)) > > + * @path: name of the file > > + * @buffer: the buffer to read the features file into (will be > > NUL-terminated on success) > > + * @size: the size of @buffer > > + * > > + * Returns: The number of bytes copied into @buffer on success (not > > counting > > + * the NUL-terminator), else -1 and errno is set. Note that @size must be > > + * larger than the size of the file or -1 will be returned with errno set > > to > > + * ENOBUFS indicating that @buffer was not large enough to contain all of > > the > > + * file contents. > > + */ > > +static int load_features_file(int dirfd, const char *path, > > + char *buffer, size_t size) > > +{ > > + autoclose int file = -1; > > + char *pos = buffer; > > + ssize_t len; > > + > > + file = openat(dirfd, path, O_RDONLY); > > + if (file < 0) { > > + PDEBUG("Could not open '%s'\n", path); > > + return -1; > > + } > > + PDEBUG("Opened features \"%s\"\n", path); > > + > > + if (!size) { > > + errno = ENOBUFS; > > + return -1; > > + } > > + > > + /* Save room for a NUL-terminator at the end of @buffer */ > > + size--; > > + > > + do { > > + len = read(file, pos, size); > > + if (len > 0) { > > + size -= len; > > + pos += len; > > + } > > + } while (len > 0 && size); > > + > > + /** > > + * Possible error conditions: > > + * - len == -1: read failed and errno is already set so return -1 > > + * - len > 0: read() never returned 0 (EOF) meaning that @buffer was > > + * too small to contain all of the file contents so set > > + * errno to ENOBUFS and return -1 > > + */ > > + if (len) { > > + if (len > 0) > > + errno = ENOBUFS; > > + > > + PDEBUG("Error reading features file '%s': %m\n", path); > > + return -1; > > + } > > + > > + /* NUL-terminate @buffer */ > > + *pos = 0; > > + > > + return pos - buffer; > > +} > > + > > static int features_dir_cb(int dirfd, const char *name, struct stat *st, > > void *data) > > { > > @@ -93,34 +156,14 @@ static int features_dir_cb(int dirfd, const char > > *name, struct stat *st, > > return -1; > > > > if (S_ISREG(st->st_mode)) { > > - autoclose int file = -1; > > int len; > > int remaining = fst->size - (fst->pos - fst->buffer); > > > > - file = openat(dirfd, name, O_RDONLY); > > - if (file == -1) { > > - PDEBUG("Could not open '%s'", name); > > - return -1; > > - } > > - PDEBUG("Opened features \"%s\"\n", name); > > - if (st->st_size > remaining) { > > - PDEBUG("Feature buffer full."); > > - errno = ENOBUFS; > > + len = load_features_file(dirfd, name, fst->pos, remaining); > > + if (len < 0) > > return -1; > > - } > > > > - do { > > - len = read(file, fst->pos, remaining); > > - if (len > 0) { > > - remaining -= len; > > - fst->pos += len; > > - *fst->pos = 0; > > - } > > - } while (len > 0); > > - if (len < 0) { > > - PDEBUG("Error reading feature file '%s'\n", name); > > - return -1; > > - } > > + fst->pos += len; > > } else if (S_ISDIR(st->st_mode)) { > > if (_aa_dirat_for_each(dirfd, name, fst, features_dir_cb)) > > return -1; > > @@ -145,27 +188,6 @@ static int handle_features_dir(const char *filename, > > char *buffer, int size, > > return 0; > > } > > > > -static int load_features_file(const char *name, char *buffer, size_t size) > > -{ > > - autofclose FILE *f = NULL; > > - size_t end; > > - > > - f = fopen(name, "r"); > > - if (!f) > > - return -1; > > - > > - errno = 0; > > - end = fread(buffer, 1, size - 1, f); > > - if (ferror(f)) { > > - if (!errno) > > - errno = EIO; > > - return -1; > > - } > > - buffer[end] = 0; > > - > > - return 0; > > -} > > - > > static bool islbrace(int c) > > { > > return c == '{'; > > @@ -362,8 +384,8 @@ int aa_features_new(aa_features **features, const char > > *path) > > > > retval = S_ISDIR(stat_file.st_mode) ? > > handle_features_dir(path, f->string, STRING_SIZE, f->string) : > > - load_features_file(path, f->string, STRING_SIZE); > > - if (retval) { > > + load_features_file(AT_FDCWD, path, f->string, STRING_SIZE); > > + if (retval == -1) { > > int save = errno; > > > > aa_features_unref(f); > > -- > -- > AppArmor mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
