From: Brian Foster <brian.foster@innova-card.com>
Date: Fri, 16 May 2008 14:07:32 GMT
Subject: Re: [PATCH] mailsplit and mailinfo: gracefully handle NUL characters
Message-ID: <a537dd660805160707y3830b164td0605a15e6ae05a5@mail.gmail.com>
In-Reply-To: <200805161539.29259.brian.foster@innova-card.com>

two quibbles of no great importance ...

Johannes Schindelin suggested:
> The function fgets() has a big problem with NUL characters: it reads
> them, but nobody will know if the NUL comes from the file stream, or
> was appended at the end of the line.
>
> So implement a custom read_line() function.
                        ^^^^^^^^^^^
                        read_line_with_nul()
meaning read part or all of one line which may contain NULs.

>[ ... ]
> diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
> index 46b27cd..021dc16 100644
> --- a/builtin-mailsplit.c
> +++ b/builtin-mailsplit.c
> @@ -45,6 +45,25 @@ static int is_from_line(const char *line, int len)
>  /* Could be as small as 64, enough to hold a Unix "From " line. */
>  static char buf[4096];
>
> +/* We cannot use fgets() because our lines can contain NULs */
> +int read_line_with_nul(char *buf, int size, FILE *in)
> +{
> +     int len = 0, c;
> +
> +     for (;;) {
> +             c = fgetc(in);
> +             buf[len++] = c;
> +             if (c == EOF || c == '\n' || len + 1 >= size)
> +                     break;
> +     }
> +
> +     if (c == EOF)
> +             len--;
> +     buf[len] = '\0';
> +
> +     return len;

 when fgetc(3) — why not use getc(3)? — returns EOF
 it is pointlessly stored in buf[] (as a 'char'!),
 len's advanced, and then the storage and advancing
 are undone.  isn't that a bit silly?   untested:

	assert(2 <= size);
	do {
		if ((c = getc(in)) == EOF)
			break;
	} while (((buf[len++] = c) != '\n' && len+1 < size);
	buf[len] = '\0'

	return len;

 I'd tend to write this in terms of pointers,
 something along the lines (untested):

	char	*p, *endp;

	assert(1 <= size);
	p    = buf;
	endp = p + (size-1);
	while (p < endp) {
		if ((c = getc(in)) == EOF || (*p++ = c) == '\n')
			break;
	}
	*p = '\0';

	return p - buf;

> +
> +}

-- 
"How many surrealists does it take to   | Brian Foster
 change a lightbulb? Three. One calms   | somewhere in south of France
 the warthog, and two fill the bathtub  |   Stop E$$o (ExxonMobil)!
 with brightly-coloured machine tools." |      http://www.stopesso.com