On Tue, Sep 06, 2016 at 08:16:03AM -0700, Christoph Hellwig wrote:
> > +/*
> > + * This is the structure used to lay out a cui log item in the
> > + * log. The cui_extents field is a variable size array whose
> > + * size is given by cui_nextents.
> > + */
> > +struct xfs_cui_log_format {
> > + __uint16_t cui_type; /* cui log item type */
> > + __uint16_t cui_size; /* size of this item */
> > + __uint32_t cui_nextents; /* # extents to free */
> > + __uint64_t cui_id; /* cui identifier */
> > + struct xfs_phys_extent cui_extents[1]; /* array of extents */
>
> Please define this as a proper variable length extent, e.g.
>
> struct xfs_phys_extent cui_extents[];
>
> and get rid of the one-off arithmentics in xfs_cui_copy_format and
> xfs_cui_init.
Ok.
> > +int
> > +xfs_cui_copy_format(
> > + struct xfs_log_iovec *buf,
> > + struct xfs_cui_log_format *dst_cui_fmt)
> > +{
> > + struct xfs_cui_log_format *src_cui_fmt;
> > + uint len;
> > +
> > + src_cui_fmt = buf->i_addr;
> > + len = sizeof(struct xfs_cui_log_format) +
> > + (src_cui_fmt->cui_nextents - 1) *
> > + sizeof(struct xfs_phys_extent);
> > +
> > + if (buf->i_len == len) {
> > + memcpy((char *)dst_cui_fmt, (char *)src_cui_fmt, len);
> > + return 0;
> > + }
> > + return -EFSCORRUPTED;
>
>
> Wouldn't life be a simpler if we simply opencoded this in
> xlog_recover_cui_pass2? Also no need for the casts in the memcpy
> arguments.
<shrug> I'm relatively indifferent either way, though I noticed that
EFIs have a separate copy_format function in xfs_extfree_item.c which
reduces the amount of clutter in xfs_log_recover.c. It seemed more
neat to keep all the log item functions corralled within a separate
file rather than putting them in xfs_log_recover.c, so I maintained
that pattern for the rui/cui/bui items.
In a semi-related side note I've been toying with turning on LTO in
xfsprogs to see how much it shrinks all the XFS code. Code size goes
down considerably (repair shrinks by nearly 1MB!) but wow the
resulting tangle can be a PITA to debug. :(
--D
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
