Re: [PATCH v2 10/12] mini-os: add struct file_ops for file type socket

[Andrew Cooper <amc96@xxxxxxxx>]

On 12/01/2022 11:32, Juergen Gross wrote:
> On 12.01.22 12:28, Andrew Cooper wrote:
>> On 11/01/2022 15:12, Juergen Gross wrote:
>>> diff --git a/lib/sys.c b/lib/sys.c
>>> index 3a8aa68..12deaed 100644
>>> --- a/lib/sys.c
>>> +++ b/lib/sys.c
>>> @@ -99,11 +99,70 @@ static struct file_ops file_ops_none = {
>>>       .name = "none",
>>>   };
>>>   +#ifdef HAVE_LWIP
>>> +static int socket_read(int fd, void *buf, size_t nbytes)
>>> +{
>>> +    return lwip_read(fd, buf, nbytes);
>>> +}
>>> +
>>> +static int socket_write(int fd, const void *buf, size_t nbytes)
>>> +{
>>> +    return lwip_write(fd, (void *)buf, nbytes);
>>> +}
>>> +
>>> +static int close_socket_fd(int fd)
>>> +{
>>> +    struct file *file = get_file_from_fd(fd);
>>> +
>>> +    return lwip_close(file->fd);
>>> +}
>>
>> Actually, on further thoughts...
>>
>>> +static struct file_ops socket_ops = {
>>> +    .name = "socket",
>>> +    .read = socket_read,
>>> +    .write = socket_write,
>>> +    .close = close_socket_fd,
>>> +    .fstat = socket_fstat,
>>> +    .fcntl = socket_fcntl,
>>> +};
>>
>> read, write and close should dispatch directly to lwip_* and not bounce
>> through a no-op local function.
>
> Not with changing the first parameter to struct file *.

Oh, good point.

>
> BTW, this patch had a bug, as the calls need to use file->fd instead of
> fd.

Yay for multiple fd spaces.

~Andrew