[R21.4] Arnaud Giersch's patch for regex.c / fix Gnus infloop

RECOMMEND 21.4 Not needed for 21.5 (it's already in 21.5; this has been a "forehead, meet palm" experience :-P ). This is Arnaud Giersch's patch to fix the Gnus infloop caused by my errors in backporting the 21.5 shy groups code. Andrey, you were looking at almost the right place! I'd appreciate it if you could check, too, although Arnaud is surely looking at the same infloop that you reported. Besides being in 21.5 and theoretically correct, Arnaud says it fixes the infloop for him. And it's a fencepost-like error, as I suggested. I'm sure this is it, although I can't test easily. This _might_ fix _some_ of the "wrong side of point" errors and so on: they could be due to accessing stale match register data, which is what this patch prevents. But I think some of those were previous to application of the backport of 21.5's shy groups, so don't get your hopes up too high. Thanks, Arnaud! I've added the ChangeLog, interchanged the initialization and the comment at the top of the block, and regularized the whitespace with `indent-region' (it's all tabs, now). Sorry, Arnaud, this will cause a conflict when you CVS update. For reference purposes, here is the semantic content of the patch (cvs diff -w src/regex.c). | Index: src/regex.c | =================================================================== | RCS file: /pack/xemacscvs/XEmacs/xemacs/src/regex.c,v | retrieving revision 1.25.2.11 | diff -u -r1.25.2.11 regex.c | --- src/regex.c 16 Jan 2005 02:38:58 -0000 1.25.2.11 | +++ src/regex.c 27 Jan 2005 08:15:23 -0000 | ＠＠ -4714,10 +4714,11 ＠＠ | succeed_label: | DEBUG_PRINT1 ("Accepting match.\n"); | | + { | /* If caller wants register contents data back, fill REGS. */ | + int num_nonshy_regs = bufp->re_nsub + 1; | if (regs && !bufp->no_sub) | { | - int num_nonshy_regs = bufp->re_nsub + 1; | /* Have the register data arrays been allocated? */ | if (bufp->regs_allocated == REGS_UNALLOCATED) | { /* No. So allocate them with malloc. We need one | ＠＠ -4801,8 +4802,9 ＠＠ | have to change the API for this function to reflect that, and | audit all callers. */ | if (regs && regs->num_regs > 0) | - for (mcnt = num_regs; mcnt < regs->num_regs; mcnt++) | + for (mcnt = num_nonshy_regs; mcnt < regs->num_regs; mcnt++) | regs->start[mcnt] = regs->end[mcnt] = -1; | + } | | DEBUG_PRINT4 ("%u failure points pushed, %u popped (%u remain).\n", | nfailure_points_pushed, nfailure_points_popped, Here are the ChangeLog and Arnaud's patch: Index: src/ChangeLog =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/ChangeLog,v retrieving revision 1.290.2.89 diff -u -U0 -r1.290.2.89 ChangeLog --- src/ChangeLog 25 Jan 2005 01:15:27 -0000 1.290.2.89 +++ src/ChangeLog 27 Jan 2005 08:13:58 -0000 ＠＠ -0,0 +1,5 ＠＠ +2005-01-26 Arnaud Giersch <arnaud.giersch(a)free.fr&gt; + + * regex.c (re_match_2_internal): + Correctly initialize loop which clears uninitialized registers. + Index: src/regex.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/regex.c,v retrieving revision 1.25.2.11 diff -u -r1.25.2.11 regex.c --- src/regex.c 16 Jan 2005 02:38:58 -0000 1.25.2.11 +++ src/regex.c 27 Jan 2005 08:24:39 -0000 ＠＠ -4714,58 +4714,59 ＠＠ succeed_label: DEBUG_PRINT1 ("Accepting match.\n"); - /* If caller wants register contents data back, fill REGS. */ - if (regs && !bufp->no_sub) - { - int num_nonshy_regs = bufp->re_nsub + 1; - /* Have the register data arrays been allocated? */ - if (bufp->regs_allocated == REGS_UNALLOCATED) - { /* No. So allocate them with malloc. We need one - extra element beyond `num_regs' for the `-1' marker - GNU code uses. */ - regs->num_regs = MAX (RE_NREGS, num_nonshy_regs + 1); - regs->start = TALLOC (regs->num_regs, regoff_t); - regs->end = TALLOC (regs->num_regs, regoff_t); - if (regs->start == NULL || regs->end == NULL) - { - FREE_VARIABLES (); - return -2; - } - bufp->regs_allocated = REGS_REALLOCATE; - } - else if (bufp->regs_allocated == REGS_REALLOCATE) - { /* Yes. If we need more elements than were already - allocated, reallocate them. If we need fewer, just - leave it alone. */ - if (regs->num_regs < num_nonshy_regs + 1) - { - regs->num_regs = num_nonshy_regs + 1; - RETALLOC (regs->start, regs->num_regs, regoff_t); - RETALLOC (regs->end, regs->num_regs, regoff_t); - if (regs->start == NULL || regs->end == NULL) - { - FREE_VARIABLES (); - return -2; - } - } - } - else - { - /* The braces fend off a "empty body in an else-statement" - warning under GCC when assert expands to nothing. */ - assert (bufp->regs_allocated == REGS_FIXED); - } - - /* Convert the pointer data in `regstart' and `regend' to - indices. Register zero has to be set differently, - since we haven't kept track of any info for it. */ - if (regs->num_regs > 0) - { - regs->start[0] = pos; - regs->end[0] = (MATCHING_IN_FIRST_STRING - ? ((regoff_t) (d - string1)) - : ((regoff_t) (d - string2 + size1))); - } + { + /* If caller wants register contents data back, fill REGS. */ + int num_nonshy_regs = bufp->re_nsub + 1; + if (regs && !bufp->no_sub) + { + /* Have the register data arrays been allocated? */ + if (bufp->regs_allocated == REGS_UNALLOCATED) + { /* No. So allocate them with malloc. We need one + extra element beyond `num_regs' for the `-1' marker + GNU code uses. */ + regs->num_regs = MAX (RE_NREGS, num_nonshy_regs + 1); + regs->start = TALLOC (regs->num_regs, regoff_t); + regs->end = TALLOC (regs->num_regs, regoff_t); + if (regs->start == NULL || regs->end == NULL) + { + FREE_VARIABLES (); + return -2; + } + bufp->regs_allocated = REGS_REALLOCATE; + } + else if (bufp->regs_allocated == REGS_REALLOCATE) + { /* Yes. If we need more elements than were already + allocated, reallocate them. If we need fewer, just + leave it alone. */ + if (regs->num_regs < num_nonshy_regs + 1) + { + regs->num_regs = num_nonshy_regs + 1; + RETALLOC (regs->start, regs->num_regs, regoff_t); + RETALLOC (regs->end, regs->num_regs, regoff_t); + if (regs->start == NULL || regs->end == NULL) + { + FREE_VARIABLES (); + return -2; + } + } + } + else + { + /* The braces fend off a "empty body in an else-statement" + warning under GCC when assert expands to nothing. */ + assert (bufp->regs_allocated == REGS_FIXED); + } + + /* Convert the pointer data in `regstart' and `regend' to + indices. Register zero has to be set differently, + since we haven't kept track of any info for it. */ + if (regs->num_regs > 0) + { + regs->start[0] = pos; + regs->end[0] = (MATCHING_IN_FIRST_STRING + ? ((regoff_t) (d - string1)) + : ((regoff_t) (d - string2 + size1))); + } /* Map over the NUM_NONSHY_REGS non-shy internal registers. Copy each into the corresponding external register. ＠＠ -4788,21 +4789,22 ＠＠ } } /* regs && !bufp->no_sub */ - /* If we have regs and the regs structure has more elements than - were in the pattern, set the extra elements to -1. If we - (re)allocated the registers, this is the case, because we - always allocate enough to have at least one -1 at the end. - - We do this even when no_sub is set because some applications - (XEmacs) reuse register structures which may contain stale - information, and permit attempts to access those registers. - - It would be possible to require the caller to do this, but we'd - have to change the API for this function to reflect that, and - audit all callers. */ - if (regs && regs->num_regs > 0) - for (mcnt = num_regs; mcnt < regs->num_regs; mcnt++) - regs->start[mcnt] = regs->end[mcnt] = -1; + /* If we have regs and the regs structure has more elements than + were in the pattern, set the extra elements to -1. If we + (re)allocated the registers, this is the case, because we + always allocate enough to have at least one -1 at the end. + + We do this even when no_sub is set because some applications + (XEmacs) reuse register structures which may contain stale + information, and permit attempts to access those registers. + + It would be possible to require the caller to do this, but we'd + have to change the API for this function to reflect that, and + audit all callers. */ + if (regs && regs->num_regs > 0) + for (mcnt = num_nonshy_regs; mcnt < regs->num_regs; mcnt++) + regs->start[mcnt] = regs->end[mcnt] = -1; + } DEBUG_PRINT4 ("%u failure points pushed, %u popped (%u remain).\n", nfailure_points_pushed, nfailure_points_popped, -- Institute of Policy and Planning Sciences http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.