Re: [RFC/WIP PATCH 1/3] teach config parsing to read from strbuf

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

Heiko Voigt <hvoigt@hvoigt.net> writes:

> diff --git a/config.c b/config.c
> index aefd80b..f995e98 100644
> --- a/config.c
> +++ b/config.c
> @@ -13,6 +13,9 @@
>  typedef struct config_file {
>  	struct config_file *prev;
>  	FILE *f;
> +	int is_strbuf;
> +	struct strbuf *strbuf_contents;
> +	int strbuf_pos;
>  	const char *name;
>  	int linenr;
>  	int eof;

The idea to allow more kinds of sources specified for "config_file"
structure is not bad per-se, but whenever you design an enhancement
to something that currently supports only on thing to allow taking
another kind, please consider what needs to be done by the next
person who adds the third kind.  That would help catch design
mistakes early.  For example, will the "string-list" (I am not
saying use of string-list makes sense as the third kind; just as an
example off the top of my head) source patch add

	int is_string_list;
        struct string_list *string_list_contents;

fields to this structure?  Sounds insane for at least two reasons:

 * if both is_strbuf and is_string_list are true, what should
   happen?

 * is there a good reason to waste storage for the three fields your
   patch adds when sring_list strage (or FILE * storage for that
   matter) is used?

The helper functions like config_fgetc() and config_ftell() sounds
like you are going in the right direction but may want to do the
OO-in-C in a similar way transport.c does, keeping a pointer to a
structure of methods, but I didn't read the remainder of this patch
very carefully enough to comment further.