On 2015-02-13 18:25:00, Seth Arnold wrote:
> On Wed, Dec 10, 2014 at 04:12:27PM -0600, Tyler Hicks wrote:
> > Signed-off-by: Tyler Hicks <[email protected]>
> 
> Acked-by: Seth Arnold <[email protected]>
> 
> A few small suggestions:
> 
> > +static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
> > +                      void *data)
> > +{
> > +   struct features_struct *fst = (struct features_struct *) data;
> > +   int remaining, len;
> > +
> > +   /* skip dot files and files with no name */
> > +   if (*name == '.' || !strlen(name))
> > +           return 0;
> 
> I don't know what a file with no name looks like but the !strlen(name)
> could be replaced by *name == '\0'.

Do you mind if I do this as a follow up patch since I'm just moving
existing code here?

> 
> > +int aa_features_new(aa_features **features, const char *path)
> > +{
> > +   struct stat stat_file;
> > +   aa_features *f;
> > +   int retval;
> > +
> > +   *features = NULL;
> > +
> > +   if (stat(path, &stat_file) == -1)
> > +           return -1;
> > +
> > +   f = (aa_features *) calloc(1, sizeof(*f));
> 
> > +int aa_features_new_from_string(aa_features **features,
> > +                           const char *string, size_t size)
> > +{
> > +   aa_features *f;
> > +
> > +   *features = NULL;
> > +
> > +   /* Require size to be less than STRING_SIZE so there's room for a NUL */
> > +   if (size >= STRING_SIZE)
> > +           return ENOBUFS;
> > +
> > +   f = (aa_features *) calloc(1, sizeof(*f));
> 
> Are these casts necessary? Normally calls for the malloc() family don't
> need casting if there is a prototype in scope, and it looked like the
> header should be included for these files.

They were necessary before this patch since this code was in the parser
and the parser uses g++. We could technically remove once the code lands
in libapparmor, which still uses gcc.

Tyler

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to