Re: [9front] [PATCH] acme: limit reads of the event file to one read.

[ori@eigenstate.org]

Definitely makes it easier to write acme clients,
I'm in favor.

Style nitpicks:

	if(foo) {, if (foo) {

should be:

	if(foo){

It's also worth asking if we should unfuck the message
format, which would be an incompatible change.

1. Always return exactly one message set, rather than
   splitting across reads
2. Counting the message size in bytes, so you can
   read and copy the required amount, rather than
   trying to figure out the byte count from the
   rune count
3. Maybe do something about the difficult-to-interpret
   continuation messages; eg, flatten them into one
   event, and put the required information into the one
   event.

I'd be ok with it, but I'm not a heavy acme user, and
I don't know how many acme users use external tools
that'd need to be fixed up.

Also, we've been more or less treating plan9port as
acme upstream, not sure if we'd want to diverge.

Quoth james palmer <james@biobuf.link>:
> this patch limits acme to one message per read of the event file.
> this makes writing clients easier because events can be read into a
> sufficiently large buffer and parsed instead of read one byte at a time.
> 
> git/import-able patch follows.
> 
> - james
> 
>  From 189797ed9098d5f6cc06cda299f1941dc07dee08
> From: james palmer <foura@biobuf.link>
> Date: Fri, 11 Jun 2021 19:41:38 +0000
> Subject: [PATCH] acme: limit reads of the events file to one event for easier parsing.
> 
> ---
> diff 3ec67240f59d1e68b8d7fcb30fa17fe87f317bff 189797ed9098d5f6cc06cda299f1941dc07dee08
> --- a/sys/src/cmd/acme/dat.h	Mon Jun  7 10:13:51 2021
> +++ b/sys/src/cmd/acme/dat.h	Fri Jun 11 20:41:38 2021
> @@ -44,6 +44,7 @@
>  typedef	struct	Fid Fid;
>  typedef	struct	File File;
>  typedef	struct	Elog Elog;
> +typedef struct	Event Event;
>  typedef	struct	Mntdir Mntdir;
>  typedef	struct	Range Range;
>  typedef	struct	Rangeset Rangeset;
> @@ -256,8 +257,8 @@
>  	int		rdselfd;
>  	Column	*col;
>  	Xfid		*eventx;
> -	char		*events;
> -	int		nevents;
> +	Event	*evhead;
> +	Event	*evtail;
>  	int		owner;
>  	int		maxlines;
>  	Dirlist	**dlp;
> @@ -278,6 +279,14 @@
>  	int		tagexpand;
>  	int		taglines;
>  	Rectangle	tagtop;
> +	int deleted;
> +};
> +
> +struct Event {
> +	char *buf;
> +	int sz;
> +	int offset;
> +	Event *next;
>  };
>  
>  void	wininit(Window*, Window*, Rectangle);
> --- a/sys/src/cmd/acme/wind.c	Mon Jun  7 10:13:51 2021
> +++ b/sys/src/cmd/acme/wind.c	Fri Jun 11 20:41:38 2021
> @@ -22,6 +22,9 @@
>  	Rune *rp;
>  	int nc, i;
>  
> +	w->deleted = 0;
> +	w->evhead = nil;
> +	w->evtail = nil;
>  	w->tag.w = w;
>  	w->taglines = 1;
>  	w->tagexpand = TRUE;
> @@ -311,6 +314,7 @@
>  winclose(Window *w)
>  {
>  	int i;
> +	Event *cur, *next;
>  
>  	if(decref(w) == 0){
>  		xfidlog(w, "del");
> @@ -322,7 +326,15 @@
>  		for(i=0; i<w->nincl; i++)
>  			free(w->incl[i]);
>  		free(w->incl);
> -		free(w->events);
> +		
> +		cur = w->evhead;
> +		while(cur) {
> +			next = cur->next;
> +			free(cur);
> +			free(cur->buf);
> +			cur = next;
> +		}
> +		
>  		free(w);
>  	}
>  }
> @@ -331,12 +343,21 @@
>  windelete(Window *w)
>  {
>  	Xfid *x;
> +	Event *cur, *next;
>  
>  	x = w->eventx;
>  	if(x){
> -		w->nevents = 0;
> -		free(w->events);
> -		w->events = nil;
> +		cur = w->evhead;
> +		while(cur) {
> +			next = cur->next;
> +			free(cur->buf);
> +			free(cur);
> +			cur = next;
> +		}
> +		
> +		w->deleted = 1;
> +		w->evhead = nil;
> +		w->evtail = nil;
>  		w->eventx = nil;
>  		sendp(x->c, nil);	/* wake him up */
>  	}
> @@ -669,26 +690,42 @@
>  void
>  winevent(Window *w, char *fmt, ...)
>  {
> -	int n;
> -	char *b;
>  	Xfid *x;
> +	char *buf;
>  	va_list arg;
> +	Event *ev;
>  
>  	if(w->nopen[QWevent] == 0)
>  		return;
>  	if(w->owner == 0)
> -		error("no window owner");
> +		error("no window owner");	
> +		
> +	ev = emalloc(sizeof(Event));
> +	ev->next = nil;
> +
>  	va_start(arg, fmt);
> -	b = vsmprint(fmt, arg);
> +	buf = vsmprint(fmt, arg);
>  	va_end(arg);
> -	if(b == nil)
> +	
> +	if(buf == nil)
>  		error("vsmprint failed");
> -	n = strlen(b);
> -	w->events = erealloc(w->events, w->nevents+1+n);
> -	w->events[w->nevents++] = w->owner;
> -	memmove(w->events+w->nevents, b, n);
> -	free(b);
> -	w->nevents += n;
> +		
> +	ev->buf = smprint("%c%s", w->owner, buf);
> +	if(!ev->buf)
> +		error("smprint failed");
> +	free(buf);
> +	
> +	ev->offset = 0;
> +	ev->sz = strlen(ev->buf);
> +	
> +	if(w->evhead) {
> +		w->evtail->next = ev;
> +		w->evtail = ev;
> +	} else {
> +		w->evhead = ev;
> +		w->evtail = ev;
> +	}
> +	
>  	x = w->eventx;
>  	if(x){
>  		w->eventx = nil;
> --- a/sys/src/cmd/acme/xfid.c	Mon Jun  7 10:13:51 2021
> +++ b/sys/src/cmd/acme/xfid.c	Fri Jun 11 20:41:38 2021
> @@ -991,34 +991,45 @@
>  xfideventread(Xfid *x, Window *w)
>  {
>  	Fcall fc;
> -	char *b;
> -	int i, n;
> +	Event *ev;
> +	int bufsz;
> +	char *buf;
>  
> -	i = 0;
>  	x->flushed = FALSE;
> -	while(w->nevents == 0){
> -		if(i){
> -			if(!x->flushed)
> -				respond(x, &fc, "window shut down");
> +	while(!w->evhead){
> +		if(w->deleted) {
> +			respond(x, &fc, "window shut down");
>  			return;
>  		}
> +		
>  		w->eventx = x;
>  		winunlock(w);
>  		recvp(x->c);
>  		winlock(w, 'F');
> -		i++;
>  	}
> -
> -	n = w->nevents;
> -	if(n > x->count)
> -		n = x->count;
> -	fc.count = n;
> -	fc.data = w->events;
> +	
> +	ev = w->evhead;
> +	bufsz = ev->sz - ev->offset < x->count
> +		? ev->sz - ev->offset : x->count;

there's a 'min()' in util.c

> +	buf = emalloc(bufsz);
> +	
> +	memcpy(buf, ev->buf+ev->offset, bufsz);
> +	ev->offset += bufsz;
> +	
> +	if(ev->offset == bufsz) {
> +		w->evhead = ev->next;
> +		free(ev->buf);
> +		free(ev);
> +		
> +		if(!w->evhead)
> +			w->evtail = nil;
> +	}
> +		
> +	
> +	fc.count = bufsz;
> +	fc.data = buf;
>  	respond(x, &fc, nil);
> -	b = w->events;
> -	w->events = estrdup(w->events+n);
> -	free(b);
> -	w->nevents -= n;
> +	free(buf);
>  }
>  
>  void
>