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

From: Jeff King <peff@peff.net>
Date: Apr 16, 2015, 18:14 UTC
Message-ID: <20150416181407.GA12517@peff.net>
In-Reply-To: <1429206774-10087-2-git-send-email-gitster@pobox.com>

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

Show 13 quoted lines

> @@ -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