Re: [Minios-devel] [UNIKRAFT PATCH] lib/vfscore: sys_ioctl() - Fix build with nolibc

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

On 9/12/19 4:40 PM, Costin Lupu wrote:
> On 9/12/19 5:32 PM, Sharan Santhanam wrote:
> > Hello Costin,
> >
> > Also noticed a checkpatch error with this patch.
> >
> > Thanks & Regards
> >
> > Sharan
> >
> >
> > On 9/11/19 6:44 PM, Costin Lupu wrote:
> > > On 9/11/19 6:27 PM, Sharan Santhanam wrote:
> > > > On 9/11/19 3:38 PM, Costin Lupu wrote:
> > > > > Hi Sharan,
> > > > >
> > > > > On 9/11/19 3:56 PM, Sharan Santhanam wrote:
> > > > > > Hello Costin,
> > > > > >
> > > > > > The fix seems fine. Please find the question inline.
> > > > > >
> > > > > > Thanks & Regards
> > > > > >
> > > > > > Sharan
> > > > > >
> > > > > > On 9/11/19 1:56 PM, Costin Lupu wrote:
> > > > > > > Commit 3dcccd04 introduced handling of FIOCLEX and FIONCLEX requests.
> > > > > > > However,
> > > > > > > these flags are not defined in nolibc.
> > > > > > >
> > > > > > > Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> > > > > > > ---
> > > > > > >      lib/nolibc/include/sys/ioctl.h | 0
> > > > > > Why do we introduce a empty file? In newlib we introduces a header
> > > > > > imported from musl.
> > > > > I didn't get that completely, does it build on your side? Do you get
> > > > > any
> > > > > errors/warnings?
> > > > No, but I guess with a review process you do clarify why you made
> > > > certain choices.
> > > > > > Why don't we use the same file here?
> > > > > This is an open question. The thing is that newlib itself seems to be a
> > > > > poor choice if we do copy so much code from musl. Now getting back to
> > > > > nolibc, if we do add more and more code from musl then we can simply
> > > > > get
> > > > > rid of it too and use musl instead. In conclusion, I fail to see why we
> > > > > should copy code to nolibc instead using musl directly.
> > > > But in this case we are introducing the FIONCLEX and FIOCLEX within the
> > > > core Unikraft and it is expected to work with nolibc. Instead of adding
> > > > #ifdef it would be better to make it feature complete.
> > > Now why would you say it is expected to work with nolibc? The two flags
> > > are actually related with close-on-exec logic. Why would any app running
> > > on top of Unikraft and which would not use a libc implementation need to
> > > call exec() since this is not possible? Besides this, there is no exec()
> > > function in nolibc.
> >  From my perspective,
> >
> > fcntl(fd, F_SETFD, FD_CLOEXEC);
> >
> > and
> >
> > ioctl(fd, FIOCLEX);
> >
> > is functionally equivalent. The former is supported in nolibc while the
> > latter is not. As far as exec call is concerned, we don't support exec
> > family in either of the libc except for glue to compile it. So when exec
> > family is available we can decide on the libcs to support the exec.
> I see, the precedent was created already. I'll send the v2 with the
> header copied from musl. To recap, I copied the header only to newlib
> glue code in the previous patch.
>
> A question that we need to answer though is until when shall we continue
> copying files from musl to both newlib and nolibc? Because this is
> getting ridiculous and non-productive.
I agree we need to better define the difference between external more 
complete libc and nolib.
> Cheers,
> Costin
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel