Re: cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery

[gs-cgit-lists.zx2c4.com@gluelogic.com]

>Steffen Nurpmeso wrote in
> <20201221193127.zbZeP%steffen at sdaoden.eu>:
> |John Keeping wrote in
> | <X+DiDgGaPaynnocI at john.keeping.me.uk>:
> ||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote:
> ||> I discovered today that cgit no longer delivers pages, and it must
> ||> have been like that for some time.  The server looks show
> ||> successful delivery, the cgit cache is populated and rotated just
> ||> correctly, but all cgit delivers is that final error of main() as
> ||> 
> ||>   <div class='error'>Error processing page: Invalid argument (22)</div>
> ...
> ||> I am pretty sure cgit delivered some weeks ago, the most notable
> ||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then
> ||> .57, which seems to have brought tremendous changes under the
> ...
> |But the file was generated normally:
> |
> |  # ll /var/lib/lighttpd/cgit/b1000000
> |  -rw-------    1 lighttpd lighttpd     23417 Dec 21 20:22 /var/lib/lighttpd/cgit/b1000000
> ...
>
>Slightly resorted:
>
> ...
> ||Have you looked at the log output?
> |  [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid argument (22)
> ...
> ||and this may be caused by sendfile(2) failing due to some difference in
> ||how the web server is setting up the output file descriptor.  You may
> ||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works.
>
>So i build it with
>
>  #alp-2020:$ diff cgit.mk.orig cgit.mk
>  --- cgit.mk.orig
>  +++ cgit.mk
>  @@ -64,7 +64,7 @@
>   endif
>
>   ifdef HAVE_LINUX_SENDFILE
>  -       CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE
>  +       #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE
>   endif
>
>   CGIT_OBJ_NAMES += cgit.o
>
>and
>
>  make NO_LUA=y NO_ICONV=y NO_GETTEXT=y NO_TCLTK=y NO_PERL=1 \
>    NO_PYTHON=1 NO_SVN_TESTS=y NO_REGEX=NeedsStartEnd prefix=/usr
>
>and .. it works.
>
>Thank you, i will open an AlpineLinux bug report.  And lighttpd.
>
>--steffen

I would like to propose an alternative and more portable solution:

cgit cache should fallback to lseek, xread, xwrite if sendfile fails.

Even if the kernel supports sendfile() and cgit is built with
HAVE_LINUX_SENDFILE defined, certain filesystem types might not support
sendfile() operations.

The following patch falls back to lseek, xread, xwrite if the *initial*
call to sendfile() fails.

diff --git a/cache.c b/cache.c
index 2c70be7..7fbab66 100644
--- a/cache.c
+++ b/cache.c
@@ -85,6 +85,7 @@ static int close_slot(struct cache_slot *slot)
 /* Print the content of the active cache slot (but skip the key). */
 static int print_slot(struct cache_slot *slot)
 {
+       ssize_t i, j;
 #ifdef HAVE_LINUX_SENDFILE
        off_t start_off;
        int ret;
@@ -97,12 +98,13 @@ static int print_slot(struct cache_slot *slot)
                if (ret < 0) {
                        if (errno == EAGAIN || errno == EINTR)
                                continue;
+                       if (start_off == slot->keylen + 1)
+                               break;
                        return errno;
                }
                return 0;
        } while (1);
-#else
-       ssize_t i, j;
+#endif
 
        i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET);
        if (i != slot->keylen + 1)
@@ -118,7 +120,6 @@ static int print_slot(struct cache_slot *slot)
                return errno;
        else
                return 0;
-#endif
 }
 
 /* Check if the slot has expired */