On 12/23/21 12:11 PM, Linus Torvalds wrote:
> On Thu, Dec 23, 2021 at 11:57 AM Stefan Roesch <shr@xxxxxx> wrote:
>>
>> + /* Attribute name */
>> + char *kname;
>> + int kname_sz;
>
> I still don't like this.
>
> Clearly the "just embed the kname in the context" didn't work, but I
> hate how this adds that "pointer and size", when the size really
> should be part of the type.
>
> The patch takes what used to be a fixed size, and turns it into
> something we pass along as an argument - for no actual good reason.
> The 'size' isn't even the size of the name, it's literally the size of
> the allocation that has a fixed definition.
>
> Can we perhaps do it another way, by just encoding the size in the
> type itself - but keeping it as a pointer.
>
> We have a fixed size for attribute names, so maybe we can do
>
> struct xattr_name {
> char name[XATTR_NAME_MAX + 1];
> };
>
> and actually use that.
>
> Because I don't see that kname_sz is ever validly anything else, and
> ever has any actual value to be passed around?
>
> Maybe some day we'd actually make that "xattr_name" structure also
> have the actual length of the name in it, but that would still *not*
> be the size of the allocation.
>
> I think it's actively misleading to have "kname_sz' that isn't
> actually the size of the name, but I also think it's stupid to have a
> variable for what is a constant value.
>
> Linus
>
Linus, I added the xattr_name struct and removed the kname_sz field from
the xattr_ctx struct. In addition the xattr_name struct is used in xattr.c
and io_uring.c.
