On Wed 13-05-15 07:50:38, Andrei Borzenkov wrote: > В Tue, 12 May 2015 15:47:40 +0200 > Jan Kara <j...@suse.cz> пишет: > > > + > > > > +static grub_uint64_t * > > > > +grub_xfs_btree_keys(struct grub_xfs_data *data, > > > > + struct grub_xfs_btree_node *leaf) > > > > +{ > > > > + char *p = (char *)(leaf + 1); > > > > + > > > > + if (data->hascrc) > > > > + p += 48; /* crc, uuid, ... */ > > > > + /* > > > > + * We have to first type to void * to avoid complaints about possible > > > > + * alignment issues on some architectures > > > > + */ > > > > + return (grub_uint64_t *)(void *)p; > > > > > > Leaving it as grub_uint64_t keys and using &keys[6] would avoid this > > > warning as well, not? Also having keys[0] will likely simplify other > > > places as well (we do require GCC in any case). > > > > Well, the trouble with this is that we'd need two structures defined - > > one for crc-enabled fs and one for old fs. That seemed like a wasted effort > > to me when we could do: > > if (data->hascrc) > > p += 48; /* crc, uuid, ... */ > > like the above. The same holds for inodes, directory entries, etc. I'd > > prefer not to bloat the code with structure definitions we don't actually > > use but if you really insisted, I could do that. So what do you think? > > Why 2 structures? What I actually meant was > > struct grub_xfs_btree_node > { > grub_uint8_t magic[4]; > grub_uint16_t level; > grub_uint16_t numrecs; > grub_uint64_t left; > grub_uint64_t right; > grub_uint64_t keys[0]; > } GRUB_PACKED; > > if (data->hascrc) > return &leaf->keys[6] > else > return &leaf->keys[0] > > with suitable comment. It is not perfect either but at least leaves > compiler check in place. Ah, I see. I probably won't call it 'keys' but something like 'data' and add a comment explaining what's going on.
Honza -- Jan Kara <j...@suse.cz> SUSE Labs, CR _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel