Subject: Re: [PATCH v3 14/25] sequencer: introduce a helper to read files written by scripts

From: Junio C Hamano <[email-masked]>
Date: Mon, 17 Oct 2016 10:17:31 -0700

Johannes Schindelin <[email-masked]> writes:

> +/*
> + * Reads a file that was presumably written by a shell script, i.e.
> + * with an end-of-line marker that needs to be stripped.
> + *
> + * Returns 1 if the file was read, 0 if it could not be read or does not exist.
> + */
> +static int read_oneliner(struct strbuf *buf,
> +	const char *path, int skip_if_empty)
> +...
> +	if (strbuf_read_file(buf, path, 0) < 0) {
> +		warning_errno(_("could not read '%s'"), path);
> +		return 0;
> +	}
> +	if (buf->len > orig_len && buf->buf[buf->len - 1] == '\n') {
> +		if (--buf->len > orig_len && buf->buf[buf->len - 1] == '\r')
> +			--buf->len;
> +		buf->buf[buf->len] = '\0';
> +	}

The name says "oneliner" but this reads the whole thing and trims
only the last line of the input.  Which is correct?

Do we want to error out if we got more than one line?  That makes it
more strict.  Going in the other direction, do we want to just read
the first line and ignore the remainder?  That allows users to leave
cruft after what matters.  I _think_ the existing code is closer to
the latter, i.e. something along the lines of ...

	struct strbuf oneline = STRBUF_INIT;
	FILE *fp = fopen(path, "r");
	if (!fp) {
		warning_errno(_("could not open '%s'"), path);
		return 0;
	}
	if (strbuf_getline(&oneline, fp) < 0)
		; /* EOF - empty */
	else {
		strbuf_addbuf(buf, &oneline);
	}