Re: [PATCH 1/5] argv-array: Add argv_array_pop function [v2]

[Junio C Hamano <gitster@pobox.com>]

Neil Horman <nhorman@tuxdriver.com> writes:

> As a convienience, it would be nice if we could pop entries off the argv_array
> structs so that if they had multiple uses in a function, we wouldn't have to
> clear them and repopulate common entries.  This patch adds the argv_array_pop
> function to do just that.  Common entries can be added to an argv_array first,
> then useage specific ones can be added on the end and removed later on.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


> CC: Jeff King <peff@peff.net>
> CC: Phil Hord <phil.hord@gmail.com>
> CC: Junio C Hamano <gitster@pobox.com>
> ---

Please don't do "Cc:" here; they belong to your e-mail header.

> diff --git a/argv-array.c b/argv-array.c
> index a4e0420..ce24a48 100644
> --- a/argv-array.c
> +++ b/argv-array.c
> @@ -39,6 +39,18 @@ void argv_array_pushf(struct argv_array *array, const char *fmt, ...)
>  	argv_array_push_nodup(array, strbuf_detach(&v, NULL));
>  }
>  
> +int argv_array_pop(struct argv_array *array, unsigned int num)
> +{
> +	if (num > array->argc)
> +		return -1;

If your use case is "After using an argv_array for the first invocation,
truncate it while keeping the common ones that appear early, so that ones
that are specific to the second invocation can be pushed", it strikes me
somewhat odd why you would want to specify "how many to pop".

Wouldn't argv_array_truncate() or argv_array_setlen() make more sense?


> +	for(num--; num>0; num--) {

Gaah.

> +		free((char **)array->argv[num]);
> +		array->argv[num] = NULL;
> +	}
> +	return 0;
> +}
> +
>  void argv_array_clear(struct argv_array *array)
>  {
>  	if (array->argv != empty_argv) {
> diff --git a/argv-array.h b/argv-array.h
> index 74dd2b1..8233243 100644
> --- a/argv-array.h
> +++ b/argv-array.h
> @@ -15,6 +15,7 @@ void argv_array_init(struct argv_array *);
>  void argv_array_push(struct argv_array *, const char *);
>  __attribute__((format (printf,2,3)))
>  void argv_array_pushf(struct argv_array *, const char *fmt, ...);
> +int argv_array_pop(struct argv_array *, unsigned int num);
>  void argv_array_clear(struct argv_array *);
>  
>  #endif /* ARGV_ARRAY_H */