Re: What's cooking in git.git (Mar 2018, #05; Wed, 28)

From: Lars Schneider <larsxschneider@gmail.com>
Date: Apr 1, 2018, 13:07 UTC
Message-ID: <27C25705-EDA7-4CD8-BBE7-F253555C4F96@gmail.com>
In-Reply-To: <B86BE818-A385-4E0C-9AE1-1E974471CEB9@gmail.com>

Show 30 quoted lines

> On 30 Mar 2018, at 12:32, Lars Schneider <larsxschneider@gmail.com> wrote:
> 
> 
>> On 30 Mar 2018, at 11:24, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> 
>> 
>> On Wed, Mar 28 2018, Junio C. Hamano wrote:
>> 
>>> * ls/checkout-encoding (2018-03-16) 10 commits
>>> - convert: add round trip check based on 'core.checkRoundtripEncoding'
>>> - convert: add tracing for 'working-tree-encoding' attribute
>>> - convert: check for detectable errors in UTF encodings
>>> - convert: add 'working-tree-encoding' attribute
>>> - utf8: add function to detect a missing UTF-16/32 BOM
>>> - utf8: add function to detect prohibited UTF-16/32 BOM
>>> - utf8: teach same_encoding() alternative UTF encoding names
>>> - strbuf: add a case insensitive starts_with()
>>> - strbuf: add xstrdup_toupper()
>>> - strbuf: remove unnecessary NUL assignment in xstrdup_tolower()
>>> 
>>> The new "checkout-encoding" attribute can ask Git to convert the
>>> contents to the specified encoding when checking out to the working
>>> tree (and the other way around when checking in).
>> 
>> There's an issue in ca16fc3635 ("convert: check for detectable errors in
>> UTF encodings", 2018-03-15) flagged by Coverity:
>> https://public-inbox.org/git/CAGZ79kbAOcwaRzjuMtZ_HVsYvUr_7UAPbOcnrmPgsdE19q=PrQ@mail.gmail.com/
> 
> Thanks a lot for pointing me at this!
> I'll prepare a new round soonish.

The report says:

  >>>     CID 1433528:  Null pointer dereferences  (FORWARD_NULL)
  >>>     Passing null pointer "src" to "validate_encoding", which dereferences it.

  411             if (validate_encoding(path, enc, src, src_len, die_on_error))

However, validate_encoding() does not dereference it. It just passes the pointer to has_prohibited_utf_bom() and is_missing_required_utf_bom(). These functions just pass the pointer to has_bom_prefix().

In has_bom_prefix() we pass the pointer to memcmp() which is undefined for null pointers. I think that is what Coverity is complaining about, right?

- Lars