Re: [PATCH v6 15/15] reftable: add print functions to the record types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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(&copy, 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux