Re: [Minios-devel] [UNIKRAFT PATCH v3 1/8] plat/linuxu: Add linuxu (x86_64) interrupts support

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Costin,


This patch emits several warnings and even errors with checkpatch.pl, 
mostly regarding formatting. These should be fixed before this can be 
pushed.

On 06/14/2018 04:12 PM, Simon Kuenzer wrote:
> --- a/plat/linuxu/include/linuxu/syscall-x86_64.h
> +++ b/plat/linuxu/include/linuxu/syscall-x86_64.h
> @@ -46,6 +46,10 @@
>   #define __SC_MUNMAP 11
>   #define __SC_IOCTL  16
>   #define __SC_EXIT   60
> +
> +#define __SC_RT_SIGACTION 13
> +#define __SC_RT_SIGPROCMASK 14
> +
>   #define __SC_PSELECT6 270
I know I'm being pedantic, but since this patch needs to be fixed 
anyway: why the introduction of empty lines in this list of defines?
> diff --git a/plat/linuxu/lcpu.c b/plat/linuxu/lcpu.c
> index 42f5690..17335c5 100644
> --- a/plat/linuxu/lcpu.c
> +++ b/plat/linuxu/lcpu.c
> @@ -33,11 +33,12 @@
>    */
>   
>   #include <stdlib.h>
> +#include <uk/plat/lcpu.h>
> +#include <_time.h>
>   #include <linuxu/syscall.h>
>   #include <uk/print.h>
> -#include <uk/plat/lcpu.h>
>   
> -void ukplat_lcpu_halt(void)
> +static void do_pselect(struct timespec *timeout)
>   {
>         int ret;
>         int nfds = 0;
> @@ -45,24 +46,22 @@ void ukplat_lcpu_halt(void)
>         fd_set *writefds = NULL;
>         fd_set *exceptfds = NULL;
>   
> -	ret = sys_pselect6(nfds, readfds, writefds, exceptfds, NULL, NULL);
> -       if (ret < 0)
> +       ret = sys_pselect6(nfds, readfds, writefds, exceptfds, timeout, NULL);
> +       if (ret < 0 && ret != -4)
>                 uk_printd(DLVL_WARN, "Failed to halt LCPU: %d\n", ret);
>   }
This magic number -4 is quite ugly. Isn't EINTR (I assume it's EINTR) 
defined anywhere? I would suggest defining it then in the correct place, 
and if that really is no option, then provide this line a comment about 
what that magic number means in that context.


--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

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