Re: [PATCH v4 06/24] nbd/client: Simplify cookie vs. index computation

[Vladimir Sementsov-Ogievskiy]

On 08.06.23 16:56, Eric Blake wrote:
> Our code relies on a sentinel cookie value of zero for deciding when a
> packet has been handled, as well as relying on array indices between 0
> and MAX_NBD_REQUESTS-1 for dereferencing purposes.  As long as we can
> symmetrically convert between two forms, there is no reason to go with
> the odd choice of using XOR with a random pointer, when we can instead
> simplify the mappings with a mere offset of 1.

Should we go further and use (uint64)-1 as a sentinel cookie value, and just 
use index as a cookie?  Or, using zero cookie in a wire looks too asymmetric?

> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

> ---
>
> v4: new patch
> ---
>   block/nbd.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index be3c46c6fee..5322e66166c 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -50,8 +50,8 @@
>   #define EN_OPTSTR ":exportname="
>   #define MAX_NBD_REQUESTS    16
>
> -#define COOKIE_TO_INDEX(bs, cookie) ((cookie) ^ (uint64_t)(intptr_t)(bs))
> -#define INDEX_TO_COOKIE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))

That looked like some security trick to hide real indices. But I don't think 
that index of request in a list is a secret information.

> +#define COOKIE_TO_INDEX(cookie) ((cookie) - 1)
> +#define INDEX_TO_COOKIE(index)  ((index) + 1)

[..]

--
Best regards,
Vladimir