Re: [PATCH 1/3] strbuf.[ch]: add STRBUF_HINT_SIZE macro = 8192

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> In b449f4cfc97 (Rework strbuf API and semantics., 2007-09-06) the
> first hardcoding of 8192 appeared in strbuf.[ch], then in
> f1696ee398e (Strbuf API extensions and fixes., 2007-09-10) another one
> was added, and in b4e04fb66e8 (strbuf: add strbuf_read_once to read
> without blocking, 2015-12-15) a third.
>
> Let's factor that out into a STRBUF_HINT_SIZE macro, and add a
> strbuf_hint() helper macro for "hint ? hint : STRBUF_HINT_SIZE".

I guess hiding 8k that appears twice in strbuf.c behind a macro or
symbolic constant might makes sense, but 

 - STRBUF_HINT_SIZE is a misnomer.  It is the default when the
   caller didn't give you a useful hint.  It is STRBUF_NO_HINT_SIZE,
   because it is the size that is used when there is no hint, or
   better yet, call it STRBUF_DEFAULT_READ_SIZE, perhaps?

 - strbuf_hint() is a misnomer for the same reason.
   strbuf_read_size(hint) perhaps, but for two or three callers,
   spelling out (hint ? hint : STRBUF_DEFAULT_READ_SIZE) is easier
   to understand.

 - Both STRBUF_HINT_SIZE and strbuf_hint() have no reason to be
   visible outside the implementation of strbuf.c; the whole reason
   why such a "the caller does not care, so let's use this logic to
   come up with the default value" exists is so that caller does not
   have to know and instead pass "0", which has nothing to do with
   the actual value used.

 - Also, there is no reason why strbuf_read() and strbuf_read_once()
   must share the same default when the caller did not give any
   hint; replacing these occurrences of 8k with a single constant
   may give a false impression that they must match.  This is merely
   an observation and not an objection, because the caller by
   passing 0 is saying they do not care at all, so while we can use
   arbitrary and different random values in these two functions, we
   can use the same arbitrary value as well.