nullfs: Use an a_gen field to cast to vop_generic_args

[def]

Summary

Instead of casting a vop_F_args object to vop_generic_args, use a
vop_F_args.a_gen field when calling null_bypass(). This way we don't
hardcode the vop_generic_args data type in the callers of null_bypass().

Before this change, there were 3 null_bypass() calls using
a vop_F_args.a_gen field and 5 null_bypass() calls using a cast to
vop_generic_args. This change makes all null_bypass() calls consistent
and easier to maintain.

Pointed out by: jrtc27

[kib]

I believe all uses of &ap->a_gen are mine, and the casts are historic.
Isn't it more reliable to avoid cast and explicitly take the address of the generic member? Either case depends on the generic being the first member of the vop_arg struct for now.

Anyway, I am curious about your choice of the cast, Is it due to the calculated number of each case in the source?

[kib]

This comment was removed by kib.

[jhb]

In D37359#849134, @kib wrote:

I believe all uses of &ap->a_gen are mine, and the casts are historic.
Isn't it more reliable to avoid cast and explicitly take the address of the generic member? Either case depends on the generic being the first member of the vop_arg struct for now.

Anyway, I am curious about your choice of the cast, Is it due to the calculated number of each case in the source?

I looked and the timestamps for use all over the place (interleaving). I slightly prefer the &gen version myself, but I think the cast was just used more times.

[def]

In D37359#849134, @kib wrote:

I believe all uses of &ap->a_gen are mine, and the casts are historic.
Isn't it more reliable to avoid cast and explicitly take the address of the generic member? Either case depends on the generic being the first member of the vop_arg struct for now.

Anyway, I am curious about your choice of the cast, Is it due to the calculated number of each case in the source?

My initial choice of the casts over the structure member was motivated by the code structure matching the intensions. An object is cast to a parent type of its structure type, rather than relies on a specific structure field that provides that functionality. Relying on the type system seemed like a more elegant solution to me.

On the other hand, one can also argue that explicitly using the structure member reflects the intensions as in this case a developer uses the field to indicate that they want to use the object as a parent structure object. However, every time the structure member name changes, all occurrences of the structure member must be updated. In case a parent structure name changes, all casts must be updated but it seems to be a justified cost since all function prototypes must be also updated in this case.

Having said that, I don't have a strong opinion what's the best choice here. If there's no strong argument for one of the choices, adapting the code to the most common scenario seems to be the right solution here.

[kib]

My preference, as you could see from the earlier response, is to have the patch in reverse direction, using &a_gen instead of cast. Cast means that you fight the type system, not utilizing it.

Anyway, I do not intend to block either change, go ahead with something.

[def]

Use an a_gen field instead of casting to vop_generic_args.

[def]

In D37359#850297, @kib wrote:

My preference, as you could see from the earlier response, is to have the patch in reverse direction, using &a_gen instead of cast. Cast means that you fight the type system, not utilizing it.

Anyway, I do not intend to block either change, go ahead with something.

Apologies for delaying this review.

I like the argument of using an a_gen field instead of hardcoding the vop_generic_args data type in the callers of nullfs_bypass(). I reversed the logic now.