git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/4] cache.h: hex2chr() - avoid -Wsign-compare warnings

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
Sep 22, 2017, 16:24 UTC
Message-ID
<d47cdae8-b37f-f378-6df6-bf70a819dd5b@ramsayjones.plus.com>
In-Reply-To
<20170922161159.hkw4id4kpnwo7jou@sigill.intra.peff.net>
On 22/09/17 17:11, Jeff King wrote:
Show 19 quoted lines
> On Fri, Sep 22, 2017 at 05:05:03PM +0100, Ramsay Jones wrote:
> 
>>> As an aside, I also see some uses of hexval() that don't appear to be
>>> quite as rigorous in checking for invalid characters. A few
>>> unconditionally shift the first nibble and assume that there will still
>>> be high bits set. I think that's generally true for twos-complement
>>> negative numbers, but isn't shifting off the left side of a signed
>>> integer undefined behavior?
>>
>> All uses of hexval() that I can see are shifting an unsigned value.
>> Have I missed something?
> 
> Hmm. get_hex_color() does:
> 
>   unsigned int val;
>   val = (hexval(in[0]) << 4) | hexval(in[1]));
> 
> Isn't that shifting the signed return value of hexval(), and then
> converting it to unsigned afterwards?

Err ... no. the return value of hexval() is *unsigned int*. (which is kinda the point!)

Show 7 quoted lines
> I've been confused by C's integer conversion rules before, though, so
> perhaps I'm wrong.
> 
> I think if this function is fed an empty string that it will also read
> past the end of the buffer for in[1]. It shouldn't matter, since the NUL
> in in[0] would cause us to return an error regardless, but it's still
> undefined behavior.
Correct.
> In fact, this whole function is just hex2chr() implemented badly. Who is
> responsible for this terrible code? ;)
;-)

ATB, Ramsay Jones

Previous: Jeff KingNext: René Scharfe
Message 7 of 8 in “cache.h: hex2chr() - avoid -Wsign-compare warnings”
  1. 3/4 cache.h: hex2chr() - avoid -Wsign-compare warningsRamsay Jones, Sep 21, 2017
  2. Jeff KingSep 22, 2017
  3. Ramsay JonesSep 22, 2017
  4. Jeff KingSep 22, 2017
  5. Jeff KingSep 22, 2017
  6. Jeff KingSep 22, 2017
  7. Ramsay JonesSep 22, 2017
  8. René ScharfeSep 22, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.