Re: [Minios-devel] [UNIKRAFT PATCH v2 1/9] include/uk/arch: Add ukarch_ffs, ukarch_fls, ukarch_flsl functions for x86_64

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi,

one drive-by comment about something I noticed while looking for 
something else:
On 08/27/2018 10:39 PM, Yuri Volchkov wrote:
> From: Costin Lupu <costin.lupu@xxxxxxxxx>
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> Reviewed-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
> ---
>   include/uk/arch/x86_64/atomic.h | 46 +++++++++++++++++++++++++++++++--
>   1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/include/uk/arch/x86_64/atomic.h b/include/uk/arch/x86_64/atomic.h
> index c5f30cc..c48d5bd 100644
> --- a/include/uk/arch/x86_64/atomic.h
> +++ b/include/uk/arch/x86_64/atomic.h
> @@ -30,6 +30,34 @@
>   #error Do not include this header directly
>   #endif
>   
> +/**
> + * ukarch_ffs - find first (lowest) set bit in word.
> + * @word: The word to search
> + *
> + * Undefined if no bit exists, so code should check against 0 first.
> + */
On this and the other functions, I'm not super happy with the fact that 
we get an undefined behavior on 0. Should we maybe do a check before the 
asm part? "If (!word) return 0;" or such? Or, at least add an 
ASSERT(!word), so that this can be caught in debug builds?
Cheers,
Florian

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel