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
