[apparmor] [PATCH v2 17/42] parser: Clean up snprintf_buffer()

[John Johansen]

On 03/06/2015 01:48 PM, Tyler Hicks wrote:
> snprintf_buffer() needed to be modified in order to properly return error
> conditions up the stack, instead of exiting, but there were some other
> cleanups that it could use.
> 
> It was obviously implemented with the features_struct in mind so this
> patch simplifies the input parameters by directly accepting a
> features_struct pointer. Also, the name is changed to reflect that it is
> intended to work on a features_struct instead of an arbritrary buffer.
> 
> A quick sanity check is added to make sure that the features_struct.pos
> value isn't pointing past the end of the buffer.
> 
> The printf(3) family of functions can return a negative value upon error
> so a check of the return value of vsnprintf(3) is added.
> 
> Finally, the return values of the function are simplified to 0 on
> success or -1, with errno set, on error. This is possible since
> features_struct.pos can be internally updated after a successful
> vsnprintf(3).
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  parser/features.c | 43 ++++++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/parser/features.c b/parser/features.c
> index 673fe4f..a2c25c2 100644
> --- a/parser/features.c
> +++ b/parser/features.c
> @@ -36,30 +36,41 @@
>  #define FEATURES_STRING_SIZE 8192
>  char *features_string = NULL;
>  
> -static char *snprintf_buffer(char *buf, char *pos, ssize_t size,
> -			     const char *fmt, ...)
> +struct features_struct {
> +	char **buffer;
> +	int size;
> +	char *pos;
> +};
> +
> +static int features_snprintf(struct features_struct *fst, const char *fmt, ...)
>  {
>  	va_list args;
> -	int i, remaining = size - (pos - buf);
> +	int i, remaining = fst->size - (fst->pos - *fst->buffer);
> +
> +	if (remaining < 0) {
> +		errno = EINVAL;
> +		PERROR(_("Invalid features buffer offset\n"));
> +		return -1;
> +	}
>  
>  	va_start(args, fmt);
> -	i = vsnprintf(pos, remaining, fmt, args);
> +	i = vsnprintf(fst->pos, remaining, fmt, args);
>  	va_end(args);
>  
> -	if (i >= size) {
> +	if (i < 0) {
> +		errno = EIO;
> +		PERROR(_("Failed to write to features buffer\n"));
> +		return -1;
> +	} else if (i >= remaining) {
> +		errno = ENOBUFS;
>  		PERROR(_("Feature buffer full."));
> -		exit(1);
> +		return -1;
>  	}
>  
> -	return pos + i;
> +	fst->pos += i;
> +	return 0;
>  }
>  
> -struct features_struct {
> -	char **buffer;
> -	int size;
> -	char *pos;
> -};
> -
>  static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
>  			   void *data)
>  {
> @@ -69,7 +80,8 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
>  	if (*name == '.' || !strlen(name))
>  		return 0;
>  
> -	fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "%s {", name);
> +	if (features_snprintf(fst, "%s {", name) == -1)
> +		return -1;
>  
>  	if (S_ISREG(st->st_mode)) {
>  		autoclose int file = -1;
> @@ -104,7 +116,8 @@ 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");
> +	if (features_snprintf(fst, "}\n") == -1)
> +		return -1;
>  
>  	return 0;
>  }
>