On Sat, Mar 06, 2021 at 04:01:10PM +0100 I heard the voice of Rhialto, and lo! it spake thus: > > I suspect the window ring used to be a normal doubly-linked list at > some time in the past. That would explain the existing checks for > NULL. With the factored out adding and removing code, there is also > more assurance that NULLs cannot sneak in.
Sounds reasonable. Might be work dropping an assert() in, just for documentation purposes, and to hint to us+5years that it's a not-how-we-thought-it-worked rather than a whoops-we-forgot-to-test, if it ever does trigger. > > - r681: WindowIsOnRing() should be static? > > If it stays there, yes, but (see next item) Well, it's just extracting some code out of an already static function for use there, so... we can always switch it to exported if we turn out to need it elsewhere, but we'd probably be moving the code then too. > Yes, I wasn't really sure where exactly to put them. > functions_internal.h is a bit of a strange header file since it > contains declarations for functions in multiple functions_*.c files, > including functions_warp.c, which by itself seems like a nice place > for window (warp) ring related stuff. Yeah, the history (and present) is a little twisted. Recall in the pre-4.x days all the f.<whatever> handling lived in menus.c (I mean, obviously, 'cuz that's what functions are, right?). I busted them out into functions.[ch] in r483, and then over the multiple steps of reworking and refining how they work into the separate functions_*.c files in r536. And part of that was just disentangling everything, so that _almost_ all of the internals could be purely internal to function bits, with the rest of the code only knowing about the things in functions.h. Which ideally would probably only be ExecuteFunction(), but in practice the event handlers and one or two other things touched a few bits that weren't convenient to try and excise, so there're a few more bits there. Then functions_internal.h could handle all the bits that needed to be shared among the internal implementations of the function handling code. In principle we could have a pile of function_xyz.h's holding it, but that'd be more work to figure what to include in each, look a bit arbitrary, and we'd _still_ have some global internal bits like the macro for the function definitions etc. So it holds things like a few internal utils and extern'd vars that wound up shared between things now in separate files, and the prototypes for the f.whatever impl's. (those latter could also in principle just be moved to being generated, since the lookup tables that call them are too, but I left 'em this way partly to easily use that to do what files they were in, and also partially as a cheap way to double test what you're doing when making changes) -- Matthew Fuller (MF4839) | fulle...@over-yonder.net Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ On the Internet, nobody can hear you scream.
