[gnutls-devel] GnuTLS | ktls: sendfile (!1486)

[Read-only notification of GnuTLS library development activities]

Merge request https://gitlab.com/gnutls/gnutls/-/merge_requests/1486 was reviewed by Daiki Ueno

--
  
Daiki Ueno started a new discussion on lib/record.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486#note_849105407

> +
> +	if (offset != NULL)
> +		if (lseek(fd, *offset, SEEK_SET) == -1)

According to the manual page of `sendfile`, the offset seems to respect the current read position so shouldn't it be `SEEK_CUR`?

--
  
Daiki Ueno started a new discussion on lib/record.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486#note_849105411

> +
> +	if (offset != NULL)
> +		if (lseek(fd, *offset, SEEK_SET) == -1)

I'd merge those `if`'s:
```suggestion:-1+0
	if (offset != NULL && lseek(fd, *offset, SEEK_SET) == -1)
```

--
  
Daiki Ueno started a new discussion on lib/record.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486#note_849105412

> +	size_t buf_len, data_to_send;
> +	size_t send = 0;
> +	char *buf;

Let's use `uint8_t *` for binary data.

--
  
Daiki Ueno started a new discussion on lib/record.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486#note_849105413

> +	if (count < buf_len)
> +		buf_len = count;
> +	else if (buf_len <= 0)

`size_t` is unsigned, so `buf_len` shouldn't be negative. In any case this clamping might be simply written using `MIN` and `MAX`?

```c
buf_len = MIN(count, MAX(MAX_RECORD_SEND_SIZE(session), 512))
```
?

--
  
Daiki Ueno started a new discussion on lib/record.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486#note_849105414

> +		buf_len = 512;
> +
> +	buf = (char*) malloc(buf_len);

Cast should be unnecessary.

--
  
Daiki Ueno started a new discussion on lib/record.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486#note_849105416

> + * @count: is the length of the data to be read from file and send.
> + *
> + * sends data via sendfile function.

If KTLS is not enabled, this function only works with blocking I/O (because of the `while` loops). Perhaps it might be worth noting that limitation something like:

```
This function sends data from @fd. If KTLS (kernel TLS) is enabled, it will use the 
sendfile() system call to avoid overhead of copying data between user space and the 
kernel. Otherwise, this functionality is only emulated by calling read() and 
gnutls_record_send() in a loop; thus it will not work in non-blocking mode. If this 
limitation matters, check whether KTLS is enabled using 
gnutls_transport_is_ktls_enabled().
```


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/1486
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20220220/a0d5318e/attachment-0001.html>