Re: [RFC PATCH 4/6] hex: label usages of hex parsing for object IDs

[Junio C Hamano <gitster@pobox.com>]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> In preparation for a future change, label the hex parsing we're doing
> for object IDs by defining a constant called HEX_KIND_OID.  This is
> currently the same as HEX_KIND_MIXED, so there is no functional change
> here.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  diagnose.c    | 2 +-
>  hex-ll.h      | 2 ++
>  hex.c         | 2 +-
>  http-push.c   | 4 ++--
>  notes.c       | 2 +-
>  object-file.c | 2 +-
>  6 files changed, 8 insertions(+), 6 deletions(-)

This is a hard-to-review patch in the sense that what we see in the
patch may be perfectly good, but we cannot see what is left out,
either by mistake or by misdesign.  So I checked out the state with
this patch (and no later ones) applied, and eyeballed the output of

    $ git grep -n -e HEX_KIND_MIXED

At this step, a few explicit uses of HEX_KIND_MIXED remain that I
think should have been converted to HEX_KIND_OID.

 * builtin/index-pack.c:repack_local_links() spawns a pack-objects
   process and reads its output.  As we are reading from a known
   version of Git (i.e., pack-objects that came with the index-pack
   that runs this code), we do not need to be lenient and can use
   HEX_KIND_OID here.

 * notes.c:load_subtree() has two calls to hex_to_bytes() to read
   paths in a notes tree, and this patch updates only one to use
   HEX_KIND_OID, leaving the other one HEX_KIND_MIXED, which we
   probably should change at the same time (if there is a valid
   reason, it deserves an in-code comment to explain it).


The remaining uses of HEX_KIND_MIXED look mostly OK.

 - color.c uses MIXED to decode things like #AAFF00, which will be
   correct forever.

 - mailinfo.c uses MIXED to decode Q encoding, and we have no power
   or business to forbid uppercase hex there.

 - pkt-line.c:packet_length() uses MIXED to decode the packet length
   expressed in the four hex digits at the beginning.  We could
   forbid uppercase hex there (our length bytes have always been
   lowercase) if we wanted to, but HEX_KIND_OID is not the enum to
   use to do so.

 - ref-filter.c:append_literal() is similar to the next one.

 - strbuf.c:strbuf_expand_literal() uses MIXED to decode %0A into line
   feed, etc.  We could forbid uppercase hex there if we wanted to,
   but HEX_KIND_OID is not the enum to use to do so.

 - url.c:url_decode_internal() uses MIXED to decode %2F into '/',
   etc., and we have no power or business to forbid uppercase hex
   there.

 - urlmatch.c:append_normalized_escapes() uses MIXED to decode %2F
   into '/' before escaping it back with %02X.