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

[Costin Lupu <costin.lup@xxxxxxxxx>]

On 9/12/19 6:01 PM, Sharan Santhanam wrote:
> 
> 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.

Sharan,

I've just sent the series for the complete porting of ioctl.h headers -
"Add ioctl.h headers to nolibc".

Cheers,
Costin

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