Capabilities functions names are super inconsistent

[SUPERCILEX]

I know deprecating stuff just for renames sucks, but the current names are way too inconsistent IMO. I'd propose the following renames (with unchanged APIs included for comparison):

• get_keep_capabilities
• set_keep_capabilities
• capabilities_secure_bits -> get_capabilities_secure_bits
• set_capabilities_secure_bits
• capability_is_in_ambient_capability_set -> is_in_ambient_capability_set
• configure_capability_in_ambient_capability_set -> configure_ambient_capability_set
• remove_capability_from_capability_bounding_set -> remove_from_capability_bounding_set
• is_in_capability_bounding_set
• clear_ambient_capability_set

[sunfishcode]

These sound reasonable to me. @koutheir, do you have any preferences about these?

If we like these names, I think it's fine to do the deprecation dance.

[koutheir]

I disagree about the renames:

• I like expressive names, because we read code more than we write code, and more than we read documentation.
• I don't add get_ prefixes when they're obvious from the name and the prototype.
• Some of the renames above make it unclear what is being checked or manipulated, and that hurts readability.

If you think the naming is inconsistent, then think about it as an opportunity to make the rest of the names better (yes, that is my subjective opinion), instead of lowering to a common denominator.

[SUPERCILEX]

@koutheir I thought these names were an improvement (writing capability 3 times is pretty unnecessary IMO), but I won't complain as long as they're consistent. What do you think of these?

• get_keep_capabilities -> keep_capabilities
• set_keep_capabilities
• capabilities_secure_bits
• set_capabilities_secure_bits
• capability_is_in_ambient_capability_set
• configure_capability_in_ambient_capability_set
• remove_capability_from_capability_bounding_set -> remove_capability_from_bounding_capability_set
• is_in_capability_bounding_set -> capability_is_in_bounding_capability_set
• clear_ambient_capability_set

[koutheir]

get_keep_capabilities -> keep_capabilities

Removing the get_ prefix from get_keep_capabilities changes the meaning of the function name because keep is a verb. A function named keep_capabilities seems to change something in order to keep capabilities, which is not what get_keep_capabilities does. I thus disagree with this renaming.

capabilities_secure_bits -> get_capabilities_secure_bits

Adding the get_ prefix does not add any meaningful information, because the absence of a verb implicitly means that the function returns that piece of data. I thus disagree with this renaming.

capability_is_in_ambient_capability_set -> is_in_ambient_capability_set

I suggest renaming to capability_is_in_ambient_set.

remove_capability_from_capability_bounding_set -> remove_from_capability_bounding_set

I suggest renaming to remove_capability_from_bounding_set.

configure_capability_in_ambient_capability_set -> configure_ambient_capability_set

I suggest renaming to configure_capability_in_ambient_set.

is_in_capability_bounding_set

I suggest renaming to capability_is_in_bounding_set.

[SUPERCILEX]

Everything except the keep/secure_bits discussion seems reasonable to me. I agree that keep_capabilities can sound a little confusing, so I think we should prefix secure_bits with get for consistency (which is more important than sounding nice IMO). So this:

• get_keep_capabilities
• set_keep_capabilities
• capabilities_secure_bits -> get_capabilities_secure_bits
• set_capabilities_secure_bits
• capability_is_in_ambient_capability_set -> capability_is_in_ambient_set
• configure_capability_in_ambient_capability_set -> configure_capability_in_ambient_set
• remove_capability_from_capability_bounding_set -> remove_capability_from_bounding_set
• is_in_capability_bounding_set -> capability_is_in_bounding_set
• clear_ambient_capability_set

[SUPERCILEX]

@koutheir Regarding capabilities_secure_bits, here's my reasoning for renaming it to get_capabilities_secure_bits:

• It is a top-level function. Unlike when a function is implemented for a struct with a field of the same name, it is unclear that a top-level function will get something.
• What's clearly a getter to some people is not clear to others. Your claim is that capabilities_secure_bits is obviously a getter. I claim that may be true for you, but not others.
• We should strive to minimize ambiguity. Adding the prefix get_ is very low-cost, but has the high reward of dropping ambiguity down to near-zero.

[koutheir]

Unlike when a function is implemented for a struct with a field of the same name, it is unclear that a top-level function will get something.

As far as I know, a function whose name starts with a verb performs that action, and a function whose name starts with a name returns that named thing. Free standing functions lack any particular context (vs the context of a struct/enum/etc.) but still follows the same naming convention.

What's clearly a getter to some people is not clear to others. Your claim is that capabilities_secure_bits is obviously a getter. I claim that may be true for you, but not others.

That is why people make conventions, to avoid surprises.

We should strive to minimize ambiguity. Adding the prefix get_ is very low-cost, but has the high reward of dropping ambiguity down to near-zero.

If we would make sure getters are always understood as such, then they all must start with get_, but that is not the convention in Rust.

[SUPERCILEX]

Hmmm, still makes me uncomfortable, but I think you're right that it's the convention in Rust. Stuff like std::fs::metadata and std::thread::current is in the stdlib.