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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux