Re: [PATCH 2/5] sed-opal: add helper for adding user authorities in ACE.

Christoph Hellwig
2023-04-04T15:25:40Z

On Wed, Mar 22, 2023 at 04:16:01PM +0100, Ondrej Kozina wrote:
> Moves ACE construction away from add_user_to_lr routine
> to be used later in added code.
>
> Signed-off-by: Ondrej Kozina <okozina@redhat.com>
> Tested-by: Luca Boccassi <bluca@debian.org>
> Tested-by: Milan Broz <gmazyland@gmail.com>
> ---
> block/sed-opal.c | 64 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index d86d3e5f5a44..2c3e38df9c65 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -1759,25 +1759,16 @@ static int set_sid_cpin_pin(struct opal_dev *dev, void *data)
> return finalize_and_send(dev, parse_and_check_status);
> }
>
> -static int add_user_to_lr(struct opal_dev *dev, void *data)
> +static int set_lr_boolean_ace(struct opal_dev *dev, unsigned int opal_uid, u8 lr,
> + const u8 *users, size_t users_len)

Please avoid the overly long line here.

> + for (u = 0; u < users_len; u++) {
> + if (users[u] == OPAL_ADMIN1)
> + memcpy(user_uid, opaluid[OPAL_ADMIN1_UID], OPAL_UID_LENGTH);
> + else {
> + memcpy(user_uid, opaluid[OPAL_USER1_UID], OPAL_UID_LENGTH);
> + user_uid[7] = users[u];
> + }
> + add_token_u8(&err, dev, OPAL_STARTNAME);
> + add_token_bytestring(&err, dev,
> + opaluid[OPAL_HALF_UID_AUTHORITY_OBJ_REF],
> + OPAL_UID_LENGTH/2);
> + add_token_bytestring(&err, dev, user_uid, OPAL_UID_LENGTH);
> + add_token_u8(&err, dev, OPAL_ENDNAME);

Please facto the logic for adding each user into a nice little helper,
which wil also avoid the overly long lines.

> + if (u > 0) {
> + add_token_u8(&err, dev, OPAL_STARTNAME);
> + add_token_bytestring(&err, dev, opaluid[OPAL_HALF_UID_BOOLEAN_ACE],
> + OPAL_UID_LENGTH/2);
> + add_token_u8(&err, dev, 1);
> + add_token_u8(&err, dev, OPAL_ENDNAME);
> + }

And this would also benefit from both a helper and a comment.