fix: read(2)ing too many bytes

this bug created this weird behaviour:

1. client 1 connects
2. client 2 connents
3. client 1 disconnects
4. client 2 sends a line
5. client 2 is disconnected by the server

client-pollin.txt

@@ -0,0 +1,50 @@
+client buffer
+
+##########++++++++A++++++A+++A++..........
+^         ^       ^      ^   ^  ^         ^
+|         |       |      |   |  |         |
+|         |       |      |   |  |         +-- end of buffer
+|         |       |      |   |  |
+|         |       |      |   |  +-- end of read(2) bytes
+|         |       |      |   |
+|         |       +-- newlines
+|         |              
+|         +-- start of line scanning
+|
++-- start of line
+
+
+# original idea
+
+- consume many lines in client__pollin
+- problem: in the echo server case, we want to write each line but a call to
+  write(2) may block, so we can't do line processing in client__pollin
+- solution: process lines in client__pollout
+
+i = scan_start
+while i < buffer_end:
+    byte = mem[i]
+    i += 1
+    if byte == '\n':
+        handle(line_start, i - line_start)
+        line_start = i
+        scan_start = i
+
+if line_start + read(2)_length >= buffer_end:
+    out_of_memory()
+    return
+
+i = line_start
+while i < line_start + read(2)_length:
+    mem[i - line_start + buffer_start] = mem[i]
+    i += 1
+
+
+# better idea
+
+in pollin handler, just read once
+indicate to the caller:
+- we read a complete line (ie 0x0a is present)
+- we read an incomplete non-empty line
+- we read nothing (EOF)
+- error from read(2)

client.s

@@ -0,0 +1,161 @@
+; Returns:
+; rax - 0
+; Errors (in rax):
+; rax - 0 - EOF
+; rax - -1024 - line too long
+; rax - other negative - from read(2)
+; Arguments:
+; rdi - fd
+; rsi - address of client within clients array
+; rdx - pollfds index
+; Variables:
+; rbx - pollfds index
+; r12 - client buffer
+; r13 - start of line scanning
+; r14 - client buffer end
+client__pollin:
+    push rbx
+    push r12
+    push r13
+    push r14
+
+    mov rbx, rdx
+    mov r12, rsi
+    add r12, OFFSET_CLIENT_BUFFER
+    mov r13, 0
+    mov r13b, [rsi + OFFSET_CLIENT_BUFFER_LEN]
+    add r13, r12
+    mov r14, r12
+    add r14, 255
+
+    mov rax, SYS_READ
+    mov rsi, r13
+    mov rdx, 255
+    sub dl, [rsi + OFFSET_CLIENT_BUFFER_LEN]
+    syscall
+
+    cmp rax, 0
+    jle client__pollin__return  ; TODO verify this shit
+
+    ; buffer_len += read(2) length
+    add [r12 - OFFSET_CLIENT_BUFFER + OFFSET_CLIENT_BUFFER_LEN], al
+
+    mov rdi, r13
+    mov rsi, rax
+    call scanline
+
+    cmp rax, 0
+    je client__pollin__no_line
+
+    mov rax, 1
+    mov word [pollfds + rbx * pollfd_size + 4], POLLOUT
+
+client__pollin__return:
+    pop r14
+    pop r13
+    pop r12
+    pop rbx
+    ret
+
+client__pollin__no_line:
+    mov r10, r13
+    add r10, rax
+    cmp r10, r14
+    jge client__pollin__line_too_long
+    mov rax, 1
+    jmp client__pollin__return
+
+client__pollin__line_too_long:
+    mov rax, -1024
+    jmp client__pollin__return
+
+
+; Returns:
+; rax - non-negative from write(2)
+; Errors:
+; rax - -1024 - no line was buffered
+; rax - other negative - from write(2)
+; Arguments:
+; rdi - fd
+; rsi - address of client within clients array
+; rdx - pollfds index
+; Variables:
+; rbx - pollfds index
+; r12 - address of client within clients array
+; r13 - line length
+; r11 - client buffer end of text after call to write(2)
+client__pollout:
+    push rbx
+    push r12
+    push r13
+
+    mov rbx, rdx
+    mov r12, rsi
+
+    push rdi
+    mov rdi, r12
+    add rdi, OFFSET_CLIENT_BUFFER
+    mov rsi, 255
+    call scanline
+    pop rdi
+
+    ; poll(2)'d for POLLOUT but we had no line buffered;
+    ; should be impossible
+    cmp rax, 0
+    je client__pollout__no_line
+
+    mov r13, rax
+
+    mov rax, SYS_WRITE
+    mov rsi, r12
+    add rsi, OFFSET_CLIENT_BUFFER
+    mov rdx, r13
+    syscall
+
+    ; error from write(2)
+    cmp rax, 0
+    jl client__pollout__return
+
+    mov r10, r12
+    mov r11, 0
+    mov r11b, [r12 + OFFSET_CLIENT_BUFFER_LEN]
+    add r11, r12
+    sub r11, rax
+
+client__pollout__shunt:
+    cmp r10, r11
+    jge client__pollout__shunt__finished
+    mov r8b, [r10 + rax]
+    mov [r10], r8b
+    add r10, 1
+    jmp client__pollout__shunt
+
+client__pollout__shunt__finished:
+    ; buffer_len -= rax
+    sub [r12 + OFFSET_CLIENT_BUFFER_LEN], al
+    cmp rax, r13
+    jl client__pollout__return
+    mov word [pollfds + rbx * pollfd_size + 4], POLLIN
+
+client__pollout__return:
+    pop r13
+    pop r12
+    pop rbx
+    ret
+
+client__pollout__no_line:
+    mov rax, -1024
+    jmp client__pollout__return
+
+; rdi - fd
+client__shutdown_close:
+    mov rax, SYS_SHUTDOWN
+    push rdi
+    mov rsi, SHUT_RDWR
+    syscall
+
+    mov rax, SYS_CLOSE
+    pop rdi
+    syscall
+
+    ret

clients.s

@@ -0,0 +1,70 @@
+; edi - fd
+clients__append:
+    ; TODO check against client_capacity
+    mov r10, [clients_len]
+    imul r10, client_size
+    add r10, clients
+    mov [r10], edi
+
+    add qword [clients_len], 1
+
+    mov r11, 4
+
+clients__append__write_zeros:
+    cmp r11, 8
+    jge return
+    mov byte [r10 + r11], 0
+    add r11, 1
+    jmp clients__append__write_zeros
+
+; edi - fd
+clients__search:
+    mov r11, [clients_len]
+    imul r11, client_size
+    add r11, clients
+    mov r10, clients
+
+clients__search__loop:
+    cmp r10, r11
+    jge clients__search__fail
+    mov r8d, [r10]
+    cmp r8d, edi
+    je clients__search__succ
+    add r10, client_size
+    jmp clients__search__loop
+
+clients__search__fail:
+    mov r10, -1
+
+clients__search__succ:
+    mov rax, r10
+    ret
+
+; rdi - address of client within clients array
+clients__remove:
+    mov r10, [clients_len]
+    cmp r10, 1
+    jle clients__clear
+
+    add r10, -1
+    mov [clients_len], r10
+
+    mov r11, r10
+    imul r11, client_size
+    add r11, clients
+
+    mov r10, 0
+
+clients__remove__loop:
+    cmp r10, client_size
+    jge return
+    mov r8b, [r11 + r10]
+    mov [rdi + r10], r8b
+    add r10, 1
+    jmp clients__remove__loop
+
+; rdi - address of client within clients array
+clients__clear:
+    mov r10, 0
+    mov [clients_len], r10
+    ret

constants.c

@@ -14,5 +14,7 @@ void main() {
     printf("sizeof(short)   %d\n", sizeof(short));
     printf("POLLIN          %d\n", POLLIN);
     printf("POLLOUT         %d\n", POLLOUT);
+    printf("POLLERR         %d\n", POLLERR);
+    printf("POLLNVAL        %d\n", POLLNVAL);
     printf("SHUT_RDWR       %d\n", SHUT_RDWR);
 }

main.s

@@ -22,10 +22,27 @@ F_SETFL         equ 4
 O_NONBLOCK      equ 2048
 POLLIN          equ 1
 POLLOUT         equ 4
+POLLERR         equ 8
+POLLNVAL        equ 32
+; TODO(?) POLLPRI: see poll(2), tcp(7)
 SHUT_RDWR       equ 2
 
-pollfd_size equ 4 + 2 + 2  ; $ man 2 poll
-pollfds_capacity equ 100
+;STATE_READING   equ 0
+;STATE_WRITING   equ 1
+
+; offsets into client struct
+OFFSET_CLIENT_FD           equ 0
+OFFSET_CLIENT_STATE        equ 4
+OFFSET_CLIENT_BUFFER_LEN   equ 5
+OFFSET_CLIENT_BUFFER       equ 6
+
+pollfd_size         equ 4 + 2 + 2  ; $ man 2 poll
+pollfds_capacity    equ 100
+
+client_buffer_size  equ 255
+; fd, state, buffer_len, buffer
+client_size         equ 4 + 2 + client_buffer_size
+clients_capacity    equ pollfds_capacity - 1
 
 SECTION .data
 server_path     db "server.sock", 0x00
@@ -33,20 +50,26 @@ server_path_len equ $ - server_path
 sockaddr_size   equ 2 + 108  ; $ man 7 unix
 goodbye         db "goodbye", 0x0a
 goodbye_len     equ $ - goodbye
+pollfds_len     dq 0
+clients_len     dq 0
 
 SECTION .bss
 sockaddr        resb sockaddr_size
 pollfds         resb pollfd_size * pollfds_capacity
-pollfds_len     resb 8
+clients         resb (4 + client_size) * clients_capacity
 
 SECTION .text
 
 %include "server.s"
+%include "readline.s"
+%include "clients.s"
+%include "pollfds.s"
+%include "client.s"
 
 _start:
     call make_server
     mov rbx, rax  ; server fd
-    mov rdi, rax
+    mov rdi, rbx
     mov rsi, POLLIN
     call pollfds__append
 
@@ -58,109 +81,115 @@ poll:
     syscall
     cmp rax, 0
     je poll
-    jl exit  ; TODO handle this gracefully
-    mov rdi, rbx
+    jl exit  ; poll(2) returned error TODO
 
-; rdi - server fd
-pollfds__scan:
+scan:
     mov r15, 0
 
-pollfds__scan__loop:
+; Variables:
+; r15 - pollfds index
+; r14w - poll(2) revents
+; r13d - fd
+; r12 - client memory address
+scan__loop:
     cmp r15, [pollfds_len]
     jge poll
-    mov r10w, [pollfds + r15 * pollfd_size + 6]
-    cmp r10w, 0
-    jne pollfds__scan__found
+    mov r14w, [pollfds + r15 * pollfd_size + 6]
+    cmp r14w, 0
+    jne scan__found
     add r15, 1
-    jmp pollfds__scan__loop
+    jmp scan__loop
 
-pollfds__scan__found:
-    mov r14, 0
-    mov r14d, [pollfds + r15 * pollfd_size]
-    cmp r14d, edi
-    je pollfds__scan__found__server
+scan__found:
+    mov r13, 0
+    mov r13d, [pollfds + r15 * pollfd_size]
+    cmp r13d, ebx
+    je scan__found__server
 
-pollfds__scan__found__client:
-    ; TODO check r10w, incl for POLLNVAL & POLLERR
-    push rdi
+scan__found__client:
+    mov rdi, r13
+    call clients__search
+    mov r12, rax
+    cmp r12, 0
+    jl _client__not_stored
 
-    mov rax, SYS_WRITE
-    mov rdi, 0
-    mov edi, r14d
-    mov rsi, goodbye
-    mov rdx, goodbye_len
-    syscall
-    cmp rax, 0
-    jl exit  ; TODO handle this gracefully
+    mov r10w, r14w
+    and r10w, POLLERR | POLLNVAL
+    cmp r10w, 0
+    jne _client__error_or_eof
 
-    mov rax, SYS_SHUTDOWN
-    mov rdi, 0
-    mov edi, r14d
-    mov rsi, SHUT_RDWR
-    syscall
+    mov r10w, r14w
+    and r10w, POLLIN
+    cmp r10w, 0
+    jne _client__pollin
 
-    mov rax, SYS_CLOSE
-    mov rdi, 0
-    mov edi, r14d
-    syscall
+    mov r10w, r14w
+    and r10w, POLLOUT
+    cmp r10w, 0
+    jne _client__pollout
 
+    ; TODO what did poll(2) detect in this case?
+    add r15, 1
+    jmp scan__loop
+
+_client__error_or_eof:
+    mov rdi, r12
+    call clients__remove
+
+_client__not_stored:
+    mov rdi, r13
+    call client__shutdown_close
     mov rdi, r15
     call pollfds__remove
+    jmp scan__loop
 
-    pop rdi
-    jmp pollfds__scan__loop
+_client__pollin:
+    mov rdi, r13
+    mov rsi, r12
+    mov rdx, r15
+    call client__pollin
+    cmp rax, 0
+    jle _client__error_or_eof
+    add r15, rax
+    jmp scan__loop
 
-pollfds__scan__found__server:
-    ; TODO check r10w, incl for POLLNVAL & POLLERR
-    push rdi
+_client__pollout:
+    mov rdi, r13
+    mov rsi, r12
+    mov rdx, r15
+    call client__pollout
+    cmp rax, 0
+    jle _client__error_or_eof
+    add r15, 1
+    jmp scan__loop
+
+scan__found__server:
+    cmp r14w, POLLIN
+    jne exit
 
     mov rax, SYS_ACCEPT
+    mov rdi, r13
     mov rsi, 0
     mov rdx, 0
     syscall
     cmp rax, 0
-    jl exit  ; TODO handle this gracefully
+    jl exit  ; accept(2) returned error TODO
 
     mov rdi, rax
-    mov si, POLLIN | POLLOUT
+    push rdi
+    mov si, POLLIN
     call pollfds__append
 
     pop rdi
+    call clients__append
+
     add r15, 1
-    jmp pollfds__scan__loop
-
-; edi - fd
-; si - events
-pollfds__append:
-    mov r10, [pollfds_len]
-    mov [pollfds + r10 * pollfd_size], edi
-    mov [pollfds + r10 * pollfd_size + 4], si
-    add r10, 1
-    mov [pollfds_len], r10
-    ret
-
-; rdi - pollfds array index to remove
-pollfds__remove:
-    mov r11, [pollfds_len]
-    cmp rdi, r10
-    jge return  ; XXX index out of bounds, do some other error?
-    cmp r11, 1
-    jle pollfds__clear
-    mov r10, [pollfds + (r11 - 1) * pollfd_size]
-    mov [pollfds + rdi * pollfd_size], r10
-    sub r11, 1
-    mov [pollfds_len], r11
-    ret
-
-pollfds__clear:
-    mov r10, 0
-    mov [pollfds_len], r10
-    ret
+    jmp scan__loop
 
 return:
     ret
 
 exit:
     mov rax, SYS_EXIT
-    mov rdi, 0
+    mov rdi, 255
     syscall

pollfds.s

@@ -0,0 +1,30 @@
+; edi - fd
+; si - events
+pollfds__append:
+    ; TODO check against pollfds_capacity
+    mov r10, [pollfds_len]
+    mov [pollfds + r10 * pollfd_size], edi
+    mov [pollfds + r10 * pollfd_size + 4], si
+    mov r11w, 0
+    mov [pollfds + r10 * pollfd_size + 6], r11w
+    add r10, 1
+    mov [pollfds_len], r10
+    ret
+
+; rdi - pollfds array index to remove
+pollfds__remove:
+    mov r11, [pollfds_len]
+    cmp rdi, r11
+    jge return  ; XXX index out of bounds, do some other error?
+    cmp r11, 1
+    jle pollfds__clear
+    mov r10, [pollfds + (r11 - 1) * pollfd_size]
+    mov [pollfds + rdi * pollfd_size], r10
+    sub r11, 1
+    mov [pollfds_len], r11
+    ret
+
+pollfds__clear:
+    mov r10, 0
+    mov [pollfds_len], r10
+    ret

readline.s

@@ -0,0 +1,20 @@
+; Returns:
+; rax - if line: line length; otherwise: 0
+; Arguments:
+; rdi - buffer
+; rsi - max length
+scanline:
+    mov rax, 0
+
+scanline__loop:
+    cmp rax, rsi
+    jge scanline__incomplete_line
+    mov r10b, [rdi + rax]
+    add rax, 1
+    cmp r10b, 0x0a  ; '\n'
+    jne scanline__loop
+    ret
+
+scanline__incomplete_line:
+    mov rax, 0
+    ret