Re: [PATCH 06/13] reftable: (de)serialization for the polymorphic record type.
- From
Han-Wen Nienhuys <hanwen@google.com>
- Date
- Sep 21, 2020, 13:13 UTC
- Message-ID
- <CAFQ2z_M9uBc+VArOVXg-hwTY8bu-gQQSL9JC6TJ5MuTCtxx=EQ@mail.gmail.com>
- In-Reply-To
- <xmqqlfh5i7nk.fsf@gitster.c.googlers.com>
On Sun, Sep 20, 2020 at 3:00 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> > +static void reftable_log_record_key(const void *r, struct strbuf *dest)
> > +{
> > + const struct reftable_log_record *rec =
> > + (const struct reftable_log_record *)r;
> > + int len = strlen(rec->refname);
> > + uint8_t i64[8];
> > + uint64_t ts = 0;
> > + strbuf_reset(dest);
> > + strbuf_add(dest, (uint8_t *)rec->refname, len + 1);
> > +
> > + ts = (~ts) - rec->update_index;
> > + put_be64(&i64[0], ts);
> > + strbuf_add(dest, i64, sizeof(i64));
> > +}
>
> We seem to be getting
>
> reftable/record.c: In function 'reftable_log_record_key':
> reftable/record.c:578:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
> put_be64(&i64[0], ts);
> ^
> CC reftable/refname.o
>
> when this series is merged to 'seen'.Thanks for bringing this up. I did see this before, but I've been unable to reproduce this locally, and I forgot about it.
The problem is actually triggered by the Git-provided put_be64() (which appears unused in the Git source code). The definition
#define put_be64(p, v) do { *(uint64_t *)(p) = htonll(v); } while (0)is trying to reinterpret a char* as a uint64_t* , which is illegal in strict aliasing rules? I originally had
+void put_be64(uint8_t *out, uint64_t v)
+{
+ int i = sizeof(uint64_t);
+ while (i--) {
+ out[i] = (uint8_t)(v & 0xff);
+ v >>= 8;
+ }
+}in my reftable library, which is portable. Is there a reason for the magic with htonll and friends?
-- 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