Re: [PATCH 1/3] utf8-bom: introduce skip_utf8_bom() helper

[Jeff King <peff@peff.net>]

On Thu, Apr 16, 2015 at 10:52:52AM -0700, Junio C Hamano wrote:

> @@ -576,10 +576,8 @@ int add_excludes_from_file_to_list(const char *fname,
>  
>  	el->filebuf = buf;
>  
> -	if (size >= 3 && !memcmp(buf, utf8_bom, 3))
> -		entry = buf + 3;
> -	else
> -		entry = buf;
> +	entry = buf;
> +	skip_utf8_bom(&entry, size);
>  
>  	for (i = 0; i < size; i++) {
>  		if (buf[i] == '\n') {

I'm surprised that in both yours and the original that we do not need to
subtract 3 from "size".

It looks like we advance "entry" here, not "buf", and then iterate over
"buf". But I think that makes the later logic weird:

   if (entry != buf + i && entry[0] != '#')

because if there is a BOM, we end up with "entry > buf + i", which I
think this code isn't expecting. I'm not sure it does anything bad, but
I think it might be simpler as just:

  /* save away the "real" copy for later, as we do now */
  el->filebuf = buf;

  /*
   * now pretend as if the BOM was not there at all by advancing
   * the pointer and shrinking the size
   */
  skip_utf8_bom(&buf, &size);

  /*
   * and now we do our usual magic with "entry"
   */
  entry = buf;
  for (i = 0; i < size; i++)
     ...

-Peff