[apparmor] [PATCH 20/31] parser: Remove prints and exits from features code

[Tyler Hicks]

On 2015-01-22 10:11:42, John Johansen wrote:
> On 12/05/2014 04:22 PM, Tyler Hicks wrote:
> > This is done in preparation for moving the features code to a library.
> > 
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> 
> I'm all for removing regular prints from the code for the library but
> do we really want to remove debug prints that only show up when the code
> is compiled with debugging enabled?

Good question. libapparmor currently has no debug logging so I continued
that tradition.

We don't know what a program linking to libapparmor will do with
stdout/stderr so I don't know if debug logging to those streams is
helpful.

Maybe in the near future we could add a debug build option to
libapparmor and have it support an env variable that specifies a file
path to open and log debug messages to?

Tyler

> 
> 
> 
> > ---
> >  parser/features.c | 33 +++++++++++++++------------------
> >  1 file changed, 15 insertions(+), 18 deletions(-)
> > 
> > diff --git a/parser/features.c b/parser/features.c
> > index 5f43c44..f3bdcd0 100644
> > --- a/parser/features.c
> > +++ b/parser/features.c
> > @@ -72,14 +72,12 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
> >  	if (S_ISREG(st->st_mode)) {
> >  		autoclose int file = -1;
> >  
> > -		if (!(file = openat(dirfd(dir), name, O_RDONLY))) {
> > -			PDEBUG("Could not open '%s'", name);
> > +		if (!(file = openat(dirfd(dir), name, O_RDONLY)))
> >  			return -1;
> > -		}
> > -		PDEBUG("Opened features \"%s\"\n", name);
> > +
> >  		remaining = fst->size - (fst->pos - fst->buffer);
> >  		if (st->st_size > remaining) {
> > -			PDEBUG("Feature buffer full.");
> > +			errno = ENOBUFS;
> >  			return -1;
> >  		}
> >  
> > @@ -91,10 +89,9 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
> >  				*fst->pos = 0;
> >  			}
> >  		} while (len > 0);
> > -		if (len < 0) {
> > -			PDEBUG("Error reading feature file '%s'\n", name);
> > +
> > +		if (len < 0)
> >  			return -1;
> > -		}
> >  	} else if (S_ISDIR(st->st_mode)) {
> >  		if (dirat_for_each(dir, name, fst, features_dir_cb))
> >  			return -1;
> > @@ -114,17 +111,15 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
> >  	return 0;
> >  }
> >  
> > -static char *handle_features_dir(const char *filename, char *buffer, int size,
> > -				 char *pos)
> > +static int handle_features_dir(const char *filename, char *buffer, int size,
> > +			       char *pos)
> >  {
> >  	struct features_struct fst = { buffer, size, pos };
> >  
> > -	if (dirat_for_each(NULL, filename, &fst, features_dir_cb)) {
> > -		PDEBUG("Failed evaluating %s\n", filename);
> > -		exit(1);
> > -	}
> > +	if (dirat_for_each(NULL, filename, &fst, features_dir_cb))
> > +		return -1;
> >  
> > -	return fst.pos;
> > +	return 0;
> >  }
> >  
> >  static int load_features_file(const char *name, char *buffer, size_t size)
> > @@ -161,6 +156,7 @@ int aa_features_new(aa_features **features, const char *path)
> >  {
> >  	struct stat stat_file;
> >  	aa_features *f;
> > +	int retval;
> >  
> >  	*features = NULL;
> >  
> > @@ -174,9 +170,10 @@ int aa_features_new(aa_features **features, const char *path)
> >  	}
> >  	aa_features_ref(f);
> >  
> > -	if (S_ISDIR(stat_file.st_mode)) {
> > -		handle_features_dir(path, f->string, STRING_SIZE, f->string);
> > -	} else if (load_features_file(path, f->string, STRING_SIZE)) {
> > +	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) {
> >  		int save = errno;
> >  
> >  		aa_features_unref(f);
> > 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150122/c288c9bc/attachment.pgp>