[apparmor] [PATCH 19/31] parser: Get rid of snprintf_buffer()

John Johansen john.johansen at canonical.com
Thu Jan 22 18:11:28 UTC 2015


On 12/05/2014 04:22 PM, Tyler Hicks wrote:
> snprintf_buffer() needed to be modified in order to properly return error
> conditions up the stack, instead of exiting, but it's added complexity
> didn't seem worth keeping it around.
> 
> This patch gets rid of that helper function and adds proper error
> handling around the new calls to snprintf().
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>

meh, I actually like removing the duplicate code, but can see the argument
against varargs ... so I suppose I am not opposed

Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  parser/features.c | 45 +++++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/parser/features.c b/parser/features.c
> index 1464ac5..5f43c44 100644
> --- a/parser/features.c
> +++ b/parser/features.c
> @@ -42,24 +42,6 @@ struct aa_features {
>  	char string[STRING_SIZE];
>  };
>  
> -static char *snprintf_buffer(char *buf, char *pos, ssize_t size,
> -			     const char *fmt, ...)
> -{
> -	va_list args;
> -	int i, remaining = size - (pos - buf);
> -
> -	va_start(args, fmt);
> -	i = vsnprintf(pos, remaining, fmt, args);
> -	va_end(args);
> -
> -	if (i >= size) {
> -		PERROR(_("Feature buffer full."));
> -		exit(1);
> -	}
> -
> -	return pos + i;
> -}
> -
>  struct features_struct {
>  	char *buffer;
>  	int size;
> @@ -70,22 +52,32 @@ 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;
>  
> -	fst->pos = snprintf_buffer(fst->buffer, fst->pos, fst->size, "%s {", name);
> +	remaining = fst->size - (fst->pos - fst->buffer);
> +	len = snprintf(fst->pos, remaining, "%s {", name);
> +	if (len == -1) {
> +		errno = EIO;
> +		return -1;
> +	} else if (len >= remaining) {
> +		errno = ENOBUFS;
> +		return -1;
> +	} else
> +		fst->pos += len;
>  
>  	if (S_ISREG(st->st_mode)) {
>  		autoclose int file = -1;
> -		int len;
> -		int remaining = fst->size - (fst->pos - fst->buffer);
> +
>  		if (!(file = openat(dirfd(dir), name, O_RDONLY))) {
>  			PDEBUG("Could not open '%s'", name);
>  			return -1;
>  		}
>  		PDEBUG("Opened features \"%s\"\n", name);
> +		remaining = fst->size - (fst->pos - fst->buffer);
>  		if (st->st_size > remaining) {
>  			PDEBUG("Feature buffer full.");
>  			return -1;
> @@ -108,7 +100,16 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
>  			return -1;
>  	}
>  
> -	fst->pos = snprintf_buffer(fst->buffer, fst->pos, fst->size, "}\n");
> +	remaining = fst->size - (fst->pos - fst->buffer);
> +	len = snprintf(fst->pos, remaining, "}\n");
> +	if (len == -1) {
> +		errno = EIO;
> +		return -1;
> +	} else if (len >= remaining) {
> +		errno = ENOBUFS;
> +		return -1;
> +	} else
> +		fst->pos += len;
>  
>  	return 0;
>  }
> 





More information about the AppArmor mailing list