From cefb204e5f12410e4f082fbdc48a9742194507f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net>
Date: Fri, 3 Jan 2025 04:21:36 -0500
Subject: [PATCH] Bring some sanity to sendfilesocket()
Not that there's any need to, it appears that it was written for
the web server, then discarded for that purpose and now is only
called from the JS Socket.sendfile() method, which always passes
NULL and 0 for the last two parameters, and has a copy/pasted
implementation for TLS sockets. The only consumer of that in
the tree appears to be gopher_service.js.
It was apparently to use as a wrapper for the high-performance
FreeBSD sendfile(), but that code behaved differently than all
the other platforms, and was disabled (behind USE_SENDFILE, which
isn't defined anywhere).
This should really just be folded into either js_socket_sendfilesocket()
or js_sendfile() with the extra knobs broken off and all the TODO
comments I'm adding here addressed.
Though, since Socket.sendfile() returns a bool where true indicates
that either the size of the file at the start of the function
was sent, or an EOF was reached, and false indicates that
"something else happened", some of the TODO comments don't really
need to be addressed.
Basically, this is a crap function and it's only used by an even
more crap JS wrapper, it should be tucked into a backward
compatibility function, removed from the JSDocs, and forgotten
about.
If Coverity keeps complaining about this, I'll wait until after
the next release and drag this out back and shoot it. Otherwise,
I'll just forget it ever existed.
---
src/xpdev/sockwrap.c | 62 +++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 36 deletions(-)
diff --git a/src/xpdev/sockwrap.c b/src/xpdev/sockwrap.c
index ed4bb4d8d1..5e0e527f05 100644
--- a/src/xpdev/sockwrap.c
+++ b/src/xpdev/sockwrap.c
@@ -153,67 +153,57 @@ socket_option_t* getSocketOptionList(void)
return(socket_options);
}
+// TODO: Only called with *offset == NULL and count == 0...
off_t sendfilesocket(int sock, int file, off_t *offset, off_t count)
{
char buf[1024*16];
off_t len;
- ssize_t rd;
- ssize_t wr=0;
off_t total=0;
- ssize_t i;
-/* sendfile() on Linux may or may not work with non-blocking sockets ToDo */
- len=filelength(file);
+ // TODO: Race condition here... length may change.
+ // But note that js_socket_sendfilesocket() reimplements all of this
+ // for encrypted sockets.
+ len = filelength(file);
if(offset!=NULL)
if(lseek(file,*offset,SEEK_SET)<0)
return(-1);
- if(count<1 || count>len) {
- count=len;
- count-=tell(file); /* don't try to read beyond EOF */
+ if (count < 1 || count > len) {
+ count = len;
+ count -= tell(file); /* don't try to read beyond EOF (why not? --- Deuce) */
}
-#if USE_SENDFILE
- while((i=sendfile(file,sock,(offset==NULL?0:*offset)+total,count-total,NULL,&wr,0))==-1 && errno==EAGAIN) {
- total+=wr;
- SLEEP(1);
- }
- if(i==0)
- return(count);
-#endif
- if(count<0) {
- errno=EINVAL;
+ if (count < 0) {
+ errno = EINVAL;
return(-1);
}
- while(total<count) {
- rd = read(file, buf, sizeof(buf));
- if (rd < 0)
+ while (total < count) {
+ ssize_t rd = read(file, buf, sizeof(buf));
+ if (rd < 0) // Error
return(-1);
- if (rd == 0)
+ if (rd == 0) // EOF
break;
- for (i = wr = 0; i < rd; i += wr) {
- wr = sendsocket(sock,buf+i,rd-i);
+ ssize_t sent = 0;
+ while (sent < rd) {
+ ssize_t wr = sendsocket(sock, buf + sent, rd - sent);
if (wr > 0) {
- if ((rd - i) < wr)
- wr = rd - i;
- continue;
+ sent += wr;
}
- if (wr == SOCKET_ERROR && SOCKET_ERRNO == EWOULDBLOCK) {
- wr = 0;
+ else if (wr == SOCKET_ERROR && SOCKET_ERRNO == EWOULDBLOCK) {
SLEEP(1);
- continue;
}
- return(wr);
+ else {
+ // TODO: This is sketchy to return 0 on write failure
+ return(wr);
+ }
}
- if(i!=rd)
- return(-1);
- total+=rd;
+ total += rd;
}
- if(offset!=NULL)
- (*offset)+=total;
+ if (offset != NULL)
+ (*offset) += total;
return(total);
}
--
GitLab