From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 10 Sep 2012 22:00:36 GMT
Subject: Re: [PATCH v2 2/6] string_list: add two new functions for splitting strings
Message-ID: <7vlighlerf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1347311926-5207-3-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> diff --git a/string-list.c b/string-list.c
> index 5594b7d..f9051ec 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -204,3 +204,52 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
>  	list->items[i] = list->items[list->nr-1];
>  	list->nr--;
>  }
> +
> +int string_list_split(struct string_list *list, const char *string,
> +		      int delim, int maxsplit)
> +{
> +	int count = 0;
> +	const char *p = string, *end;
> +
> +	assert(list->strdup_strings);

This may be a taste thing, but I'd prefer to see assert() only for
verification of pre-condition by internal callers to catch stupid
programming errors.  For a library-ish function like sl_split() that
expects to be called from anybody outside the string-list API, a
violation of this pre-condition is a usage error of the API, and
should trigger a runtime error (even without NDEBUG), no?