[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    Re: [PATCH,v2] [AArch64] MTE corefile support
From:       Luis Machado via Gdb-patches <gdb-patches () sourceware ! org>
Date:       2021-05-31 14:56:53
Message-ID: 860e15d8-ed55-d5de-6d72-1aedd9a84f93 () linaro ! org
[Download RAW message or body]

On 5/31/21 11:49 AM, Simon Marchi wrote:
> On 2021-05-31 10:12 a.m., Luis Machado wrote:
> > > > @@ -71,4 +72,18 @@ extern CORE_ADDR aarch64_mte_set_ltag (CORE_ADDR address, \
> > > > CORE_ADDR tag); It is always possible to get the logical tag.  */
> > > > extern CORE_ADDR aarch64_mte_get_ltag (CORE_ADDR address);
> > > > +/* MTE-specific NT_MEMTAG header.  */
> > > > +struct tag_dump_mte
> > > > +{
> > > > +  /* Size of the tag granule in bytes.  */
> > > > +  uint16_t granule_byte_size;
> > > > +  /* Size of the tag in bits.  */
> > > > +  uint16_t tag_bit_size;
> > > > +  /* Reserved field for the future.  */
> > > > +  uint16_t __unused;
> > > > +};
> > > 
> > > This struct is unused.
> > > 
> > 
> > I'm inclined to leave that as documentation instead, so it is clear what the \
> > header sizes are referring to.
> 
> The thing is that somebody reading the code in the .c seeing the
> arbitrary sizeofs  will note have a clue to go look at
> arch/aarch64-mte-linux.h for explanations.  So another option would be
> to write it as a comment near the code.  Something like:
> 
> /* The header follows the following format:
> 
> struct
> {
> /* Size of the tag granule in bytes.  */
> uint16_t granule_byte_size;
> /* Size of the tag in bits.  */
> uint16_t tag_bit_size;
> /* Reserved field for the future.  */
> uint16_t __unused;
> };
> */
> 
> Another option would be to have the structure and refer to its fields in
> sizeof:
> 
> store_unsigned_integer (buf, sizeof (tag_dump_mte::granule_byte_size),
> 			    byte_order, AARCH64_MTE_GRANULE_SIZE);

I think that would work as well. I can change it for the next iteration.


[prev in list] [next in list] [prev in thread] [next in thread]