Skip to content

Commit

Permalink
Update sa_lengths properly when replacing variable-size SA.
Browse files Browse the repository at this point in the history 
When updating a SA, the sa_lengths array is used to track the sizes
of all variable-sized SAs.  It had not been properly updated when
copying an already-existing variable-sized SA.  This didn't come up too
often because variable-sized SAs are relatively rare.

Fixes openzfs#1978.
  • Loading branch information
@dweeezil
dweeezil committed Dec 19, 2013
1 parent c2d439d commit 734d677
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion module/zfs/sa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1740,10 +1740,11 @@ sa_modify_attrs(sa_handle_t *hdl, sa_attr_type_t newattr,
j++;
continue;
}
ASSERT(SA_REGISTERED_LEN(sa, attr) == 0);
length = SA_REGISTERED_LEN(sa, attr);

This comment has been minimized.

Sign in to view

Copy link
@chrisrd

chrisrd Dec 19, 2013

I run with asserts enabled and I've not hit that ASSERT() - and length isn't used. Is that some debugging that's leaked into the patch? Otherwise, if the ASSERT() shouldn't be there, then the line can be removed altogether?
ASSERT(action == SA_REPLACE);
SA_ADD_BULK_ATTR(attr_desc, j, attr,
locator, datastart, buflen);
hdr->sa_lengths[length_idx++] = buflen;
} else {
length = SA_REGISTERED_LEN(sa, attr);
if (length == 0) {
Expand Down

1 comment on commit 734d677

@chrisrd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst we're here... this part a bit further down also looks odd:

        if (action == SA_ADD) {
                length = SA_REGISTERED_LEN(sa, newattr);
                if (length == 0) {
                        length = buflen;
                }
                SA_ADD_BULK_ATTR(attr_desc, j, newattr, locator,
                    datastart, buflen);
        }

Once again length isn't used?? I.e. the above is equivalent to:

        if (action == SA_ADD) {
                SA_ADD_BULK_ATTR(attr_desc, j, newattr, locator,
                    datastart, buflen);
        }

Please sign in to comment.