Florian Obser wrotes: >On Mon, Jan 02, 2017 at 04:29:21PM +0330, temp+...@frad.ir wrote: >> Hi tech@, >> >> I recently checked the slowcgi(8) and found that it might have an issue >> when buf_pos is at the end of buffer and buf_len is zero. >> >> Am I right? > >we can simplify this even more. There is no need to remember the >buffer position outside of this function. It will be 0 on every call, >either because we made progress in parsing data and then copied the >rest to the beginning of the buffer or we did not make progress at >all, and then we need to start parsing from the beginning again. > >OK?
It seems perfect. > >diff --git slowcgi.c slowcgi.c >index dec4df8d1a1..83d12d99160 100644 >--- slowcgi.c >+++ slowcgi.c >@@ -118,7 +118,6 @@ struct request { > struct event tmo; > int fd; > uint8_t buf[FCGI_RECORD_SIZE]; >- size_t buf_pos; > size_t buf_len; > struct fcgi_response_head response_head; > struct fcgi_stdin_head stdin_head; >@@ -495,7 +494,6 @@ slowcgi_accept(int fd, short events, void *arg) > return; > } > c->fd = s; >- c->buf_pos = 0; > c->buf_len = 0; > c->request_started = 0; > c->stdin_fd_closed = c->stdout_fd_closed = c->stderr_fd_closed = 0; >@@ -632,12 +630,12 @@ slowcgi_request(int fd, short events, void *arg) > { > struct request *c; > ssize_t n; >- size_t parsed; >+ size_t parsed, pos = 0; > > c = arg; > >- n = read(fd, c->buf + c->buf_pos + c->buf_len, >- FCGI_RECORD_SIZE - c->buf_pos-c->buf_len); >+ n = read(fd, c->buf + c->buf_len, >+ FCGI_RECORD_SIZE - c->buf_len); > > switch (n) { > case -1: >@@ -666,16 +664,15 @@ slowcgi_request(int fd, short events, void *arg) > * at that point, which is what happens here. > */ > do { >- parsed = parse_record(c->buf + c->buf_pos, c->buf_len, c); >- c->buf_pos += parsed; >+ parsed = parse_record(c->buf + pos, c->buf_len, c); >+ pos += parsed; > c->buf_len -= parsed; > } while (parsed > 0 && c->buf_len > 0); > > /* Make space for further reads */ >- if (c->buf_len > 0) { >- bcopy(c->buf + c->buf_pos, c->buf, c->buf_len); >- c->buf_pos = 0; >- } >+ if (c->buf_len > 0 && pos > 0) >+ bcopy(c->buf + pos, c->buf, c->buf_len); >+ > return; > fail: > cleanup_request(c); > >