Re: [PATCH v4 02/17] xfs: Replace attribute parameters with struct xfs_name

Allison Collins
2019-11-11T20:08:12Z

On 11/11/19 10:49 AM, Christoph Hellwig wrote:
> On Wed, Nov 06, 2019 at 06:27:46PM -0700, Allison Collins wrote:
>> This patch replaces the attribute name and length parameters with a
>> single struct xfs_name parameter. This helps to clean up the numbers of
>> parameters being passed around and pre-simplifies the code some.
>
> Does it? As-is the patch doesn't look too bad, although it adds more
> lines than it removes. But together with the next patch that embedds
> an xfs_name into the xfs_da_args structure and the now added memcpys
> it doesn't really look very helpful to me.
>
It has evolved a bit since its initial proposition. Maybe it would help
to raise the question of: is patch 2 or 3 alone more helpful than the
combination of them? I think initially Dave had suggested the clean up
in one of the earlier reviews a while ago, so he may have an opinion here.

Personally I'm not super opinionated about it if people have strong
feelings toward keeping or pitching it. I will say after having worked
with it as it is, I'm not sure I'm particularly fond of having a name
struct in the param, and another in the args. I think eventually
someones going to forget that args supersedes the param after the init
routine. People will probably figure it out pretty quick if they get
bit I suppose, but I do think it's a sort of wart to consider.

Allison