On Fri, Jan 21, 2022 at 1:40 PM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> > +static void reftable_index_record_print(const void *rec, int hash_size)
> > +{
> > + const struct reftable_index_record *idx = rec;
> > + /* TODO: escape null chars? */
>
> That's quite scary as a TODO comment, can we think about whether we're
> doing the wrong thing here? I.e. the printf() will stop on a \0, or
> maybe I'm misunderstanding this...
It's a print function, so we'd print a truncated last_key. This can
happen if we were printing an index record that points to a log
record. I'd address this if and when I have to debug into indexing
going wrong (which hasn't happened so far.)
> > /* Are two records equal? This assumes they have the same type. Returns 0 for non-equal. */
> > int (*equal)(const void *a, const void *b, int hash_size);
> > +
> > + /* Print on stdout, for debugging. */
>
> Inaccurate comment, this isn't for ad-hoc debugging, we're using this in
> the reftable tests....
The tests say
> > + puts("testing print coverage:\n");
> > + reftable_record_print(©, GIT_SHA1_RAWSZ);
ie. I'm calling the print functions to make sure they don't crash or
leak memory. They're not used as part of functionality of either the
reftable code or its tests.
> > + .old_hash = reftable_malloc(GIT_SHA1_RAWSZ),
> > + .new_hash = reftable_malloc(GIT_SHA1_RAWSZ),
> > + /* rest of fields left empty. */
>
> But in test_reftable_log_record_roundtrip() we have another existing
> user who leaves out fields, but doesn't have such a comment.
rephrased comment.
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
