Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
