Re: [9front] [patch] skelfs: remove useless code

[Jacob Moody <moody@mail.posixcafe.org>]

On 11/11/22 10:52, Arne Meyer wrote:
> After that patch I grepped through /sys/src/cmd/* and found a more hits. Is there any difference between handrolled emalloc implementations and emalloc9p? If not, I can go spelunking to replace emalloc where emalloc9p is available.
> 
>> Jacob Moody <moody@mail.posixcafe.org> hat am 11.11.2022 17:37 GMT geschrieben:
>>
>>  
>> On 11/11/22 10:17, Arne Meyer wrote:
>>> Hello,
>>>   
>>> if I read the code correctly emalloc9p already checks for failed allocations and zeroes the memory, so the following code is redundant.
>>>
>>> Greetings,
>>> Arne
>>
>> You are correct, muscle memory did not work in my favor this time.
>>
>> Thanks!
>> moody

The only difference I generally see is that the emalloc9p and friends will call exits("mem"), which in the context
of lib9p (generally) just means the process for that specific pipe is killed. I believe the intention here is to allow other srv
procs to continue existing, but if that is the case we surely are doing it half assed right now.

For starters we should probably be using threadexits() when we're threaded, and we're surely leaking entries in the
fidmaps and the Fid structs themselves. Ideally those functions would be given enough context to properly unroll
the entire process state, if we did want to really handle this case. Of course this is only for file systems
that do have that listen/fork loop, if you are just posting to /srv then you will only ever have the one service
proc. But then of course for threaded systems you have to communicate that things blew up, or else your srv
proc could just die and leave everything else deadlocked.

So here's my shot in the dark for something that could be a bit better, if we had the e* functions in mem.c
take a Request as the first argument, then perhaps provide a destroysrv callback for user code to clean things up
associated with that pipe. With that those functions have a better idea of how to clean up the mess. But these error
handling unrolling things are almost all some brand of awful, it may not be worth the effort to do 'better'.

While reviewing the lib9p code for these thoughts, I came across a scenario of lib9p checking for this
stuff itself, this changes that:

diff a39eb30ffa2138a817bd787fdfd25eb65927e7d4 uncommitted
--- a//sys/src/lib9p/srv.c
+++ b//sys/src/lib9p/srv.c
@@ -666,10 +666,6 @@
 	}
 	n = GBIT16(tmp)+BIT16SZ;
 	statbuf = emalloc9p(n);
-	if(statbuf == nil){
-		r->error = "out of memory";
-		return;
-	}
 	r->ofcall.nstat = convD2M(&r->d, statbuf, n);
 	r->ofcall.stat = statbuf;	/* freed in closereq */
 	if(r->ofcall.nstat <= BIT16SZ){