Change behaviour of sendfile(2) on FreeBSD to mimic Linux

[shamazmazum]

Hi!

I think it would be a great idea to change lfp_sendfile on FreeBSD and derivatives so it returns number of bytes sent, not just 0 or -1. As manual says, this is exactly what Linux'es sendfile does.

Also I think there is no need to treat EAGAIN as an error because, unlike with write or send, it is an usual error code that sendfile returns even if a file is partially sent.

Finally, I remove obsolete SF_MNOWAIT flag.

[shamazmazum]

An excerpt from sendfile(2) manpage concerning EAGAIN

[EAGAIN]
The socket is marked for non-blocking I/O and not all data was sent due to the 
socket buffer being filled.  If specified, the number of bytes successfully sent 
will be returned in *sbytes

[shamazmazum]

Hi! I thought this project is dead, but got an email from github showing me it's not dead completely. Why this patch is without response for 2 years?

[sionescu]

Lack of time. I plan to go through the pull requests "soon".

[jmercouris]

@sionescu any update?

[nueidris]

@shamazmazum are you okay if I submit the patch to FreeBSD ports tree?

[shamazmazum]

@shiorid Sounds great!

[shamazmazum]

@sionescu Just checked, you actually closed my PR and pushed some shit which does not fix the problem. What's wrong with you? Spent some time to figure why my shitty HTML server does not work. Turns out my patch just disappeared.

@shiorid Are you a maintainer of libfixposix in the ports tree? Can you push my version? Check the manual if in doubt.

[shamazmazum]

@sionescu If you simply do not have time, I can be a maintainer. But just closing a PR and replacing a fix with something else is not OK.

[sionescu]

@shamazmazum If my change is faulty, please let me know why and I'll fix it.
And if you come here with this language again, you'll get banned.

[nueidris]

@shiorid Are you a maintainer of libfixposix in the ports tree? Can you push my version? Check the manual if in doubt.

Yes, I'm the maintainer and will submit the changes to public review (since I'm not a committer).

Any way, I'm not a great C programmer, so could you confirm if the problem on the upstreamed “fix” lives here?

if (res == 0) { return sbytes; }
return res;

[shamazmazum]

if my change is faulty, please let me know why and I'll fix it.

I wrote specially about EAGAIN case. I understand that not everyone has FreeBSD installed, so I copied that part of the manual for you.

And if you come here with this language again, you'll get banned.

This is your repository after all, you can do whatever you wish.

[shamazmazum]

@shiorid

My original patch was:

- # elif defined(__FreeBSD__)
-    return (ssize_t) sendfile(in_fd, out_fd, offset, nbytes, NULL, NULL, SF_MNOWAIT);
+ # elif defined(__FreeBSD__) || defined(__DragonFly__)
+    off_t sbytes;
+    int res = sendfile(in_fd, out_fd, offset, nbytes, NULL, &sbytes, 0);
+    if (res == -1 && errno == EAGAIN)
+        res = 0;
+    if (res == 0) res = sbytes;
+    return res;

EAGAIN is not actually an error on FreeBSD. The number of bytes send will be present in sbytes.

[sionescu]

The usual semantics of EAGAIN is that no I/O was performed because the operation would have blocked. You mean that on FreeBSD that's not the case ?

[shamazmazum]

Exactly. See here or in my second comment. In FreeBSD EAGAIN means that operation was performed only partially.

[nueidris]

EAGAIN is not actually an error on FreeBSD. The number of bytes send will be present in sbytes.

@shamazmazum thank you for the explanation.

Since 0.5.0 is not yet released, I'll cherry-pick your fix and bump the revision in ports tree.

Thank you for letting me know.

[sionescu]

I've committed cc50634.

[shamazmazum]

Thanks. I can confirm that it works now.

[shamazmazum]

Not if I was interested, but are you sure that __APPLE__ has the same behaviour?

[sionescu]

The official man page says so:

When using a socket marked for non-blocking I/O, sendfile() may send fewer bytes than requested. In this case, the number of bytes success-fully successfully fully sent is returned in the via the len parameters and the error EAGAIN is returned.

[shamazmazum]

Oh, OK. Thanks

[shamazmazum]

BTW, you did it incorrect again. Look at my patch:

int res = sendfile(in_fd, out_fd, offset, nbytes, NULL, &sbytes, 0);
    if (res == -1 && errno == EAGAIN)
        res = 0;
    if (res == 0) res = sbytes;
    return res;

and at what you did:

    off_t sbytes = 0;
    int res = sendfile(in_fd, out_fd, offset, nbytes, NULL, &sbytes, 0);
    if (res == 0 || (res < 0 && errno == EAGAIN && sbytes > 0)) {
        return sbytes;
    }
    return -1;

Don't you see a corner case? Hint: look what happens when sbytes == 0. I really cannot understand why you continue to ignore a correct patch and write your own "fixes" (this will be third) instead. You just got offended when I asked the first time. Didn't want to offend you, but please stay close to the manual.

[sionescu]

The purpose of lfp_sendfile() is to emulate the semantics of the Linux sendfile(), which are as follows:

• if zero bytes were written, it returns an error and sets errno to EAGAIN
• otherwise, the number of written bytes is returned, and a short write is detected by comparing the return value with the requested number of bytes

In other words, the current treatment of EAGAIN with sbytes==0 is correct, and it's what I meant. Your patch was incorrect.

[shamazmazum]

OK, I am wrong, sorry. It wasn't clear for me what is considered a successful transfer on Linux (see here):

  If the transfer was successful, the number of bytes written to
       out_fd is returned.  Note that a successful call to sendfile()
       may write fewer bytes than requested; the caller should be
       prepared to retry the call if there were unsent bytes.  See also
       NOTES.

       On error, -1 is returned, and errno is set to indicate the error.

I thought transfering zero bytes can also be considered successful (it's not clear then, in what cases EAGAIN should be returned as an error). It seems that returning zero has a different semantics on Linux and FreeBSD (here is a discussion). My original intent was to mask EAGAIN because on FreeBSD it's not an error, actually. But I see I am wrong and on Linux returning zero and returning -1+EAGAIN are different things.

[nueidris]

hey, just to make you know, the fix is on ports tree.

Cheers!

[sionescu]

@shamazmazum a return of 0 means EOF.