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

Allison Collins
2019-11-13T15:13:09Z

On 11/11/19 1:07 PM, Allison Collins wrote:
>
>
> 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

What if I added an xfs_name_init to reduce LOC. and then dropped patch
3? What would people think of that?

Allison