On Tue, Sep 23, 2008 at 00:15, Tony Balinski <[EMAIL PROTECTED]> wrote: > Quoting Bert Wesarg <[EMAIL PROTECTED]>: > >> This is my proposal for fixing several issues with the handling of compiled >> macro programs. > ... >> If there are no objections, I commit this by the end of the week, plus my >> proposal B for SF: BUG#2074318, from here: > > setupFrame() doesn't check the remaining space in the stack to see if the > call frame will fit. In my latest stuff, I just folded all stack frame > building into a routine, callSubroutineFromSymbol(), which uses the current > context so that the PUSH macros etc will do the check (I added a PUSH_CHECK > macro to allow checking for the function linkage space in one go). For > rewindFrame() checking is less important: all should be correct already.
I added these check macros to my 'interpre.c macro cleanup' patch. The 'problem' (it isn't really a problem only the current situation) is, that the POP/PUSH macros all work on the global StackP variable, which make these useless for setupFrame(). I totally agree that any code that setup the next function frame should at lest check the stack size. Part of the current problem (and you have realized this long before: +/* TODO: this function should really return a status (if fails PUSH_CHECK) */ void RunMacroAsSubrCall(Program *prog) ) is that not all frame setup sites handle a stack error. > Why not have it take a RestartData *context argument BTW? setupFrame() and rewindFrame() are RestartData agnostic, so I decided that rewindStack() should be agnostic too. Despite the fact, that it is currently only called from FreeRestartData(). Ohh, I see that you asked for rewindFrame(), thats RestartData agnostic, because it is also called from returnValOrNone(), which hasn't a RestartData. > > setupFrame() also checks for a negative args count. That's an extension I > added for the named arguments/arguments array stuff. You shouldn't need it in > the code base (yet!). I know this but they don't collide. What collides is the haveNamedArg, which is flagged by your negative nArgs. But if I call setupFrame() you have removed your negative flag from nArgs already. I could also use a flag parameter and combine the passProg and argsOnStack flags into this. And in your anonArray patch you add a new flag for haveNamedArg. I have changed your patches to match this patch, you may have a look at them here: InterpretDebug4.diff: http://repo.or.cz/w/nedit-bw.git?a=blob;f=InterpretDebug4.diff;h=6825888910dfc05b795348e4ec6fb5192bd8213e;hb=0a73d0479a0001cf960567beb09c16a55ef0c7bf anonArrayNamedArgs7.diff: http://repo.or.cz/w/nedit-bw.git?a=blob;f=anonArrayNamedArgs7.diff;h=05334585e33fc8cd09154931518a309c06591cb3;hb=0a73d0479a0001cf960567beb09c16a55ef0c7bf callMacroFnByName2.diff: http://repo.or.cz/w/nedit-bw.git?a=blob;f=callMacroFnByName2.diff;h=23e1db36678a7fa05b7406e11064f166131653bc;hb=0a73d0479a0001cf960567beb09c16a55ef0c7bf > > Other than that, having these two functions defined close together is a very > sound idea. And it's good that the same operations aren't repeated 3 times > over (as in the current source base). > > As for your 2074318 stuff, why not? Because I posted two proposals, proposal A would just bail out if more that 9 arguments are passed. > > All in all, I'm for it. Does anyone else have anything to say? This would be > for the v5.6 ("L'Arlesienne") version, I suppose! Hmm, my french was never good, but it is probably not a positive word. Bert > > Tony > -- > NEdit Develop mailing list - [email protected] > http://www.nedit.org/mailman/listinfo/develop > -- NEdit Develop mailing list - [email protected] http://www.nedit.org/mailman/listinfo/develop
