X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f Sender: rich AT phekda DOT freeserve DOT co DOT uk Message-ID: <3FAEC29A.2E7F6EF9@phekda.freeserve.co.uk> Date: Sun, 09 Nov 2003 22:41:30 +0000 From: Richard Dawe X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.2.23 i586) X-Accept-Language: de,fr MIME-Version: 1.0 To: djgpp-workers AT delorie DOT com Subject: Re: regcomp.c realloc fix References: <3F9E29ED DOT 000004 DOT 02439 AT camay DOT yandex DOT ru> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Note-from-DJ: This may be spam Reply-To: djgpp-workers AT delorie DOT com Hello. Alexander Aganichev wrote: > > OK, let's start from fixing regexp library. The what I fixed there can be > split in 5 patches, which I will post one-by-one until we fix everything. > It is includes: > 1) realloc fix Thanks! > (BTW, should we rescan current tree for unsafe realloc's > again and fix everything?) We should probably recheck the tree. I have some realloc fixes that I can send you, if you like. I sent them to djgpp-workers a while ago. I haven't tried to get them into CVS, because they are untested or pointless. An example of a pointless fix is to the start-up code: if a memory allocation fails there, then the program is unlikely to get very far. I have some comments about the patch. See inline below. [snip] > Here is fix for regcomp.c for accurate realloc handling: > > diff -rup djgpp.orig/src/libc/posix/regex/regcomp.c djgpp/src/libc/posix/regex/regcomp.c > --- djgpp.orig/src/libc/posix/regex/regcomp.c 2002-10-18 04:08:00.000000000 +0000 > +++ djgpp/src/libc/posix/regex/regcomp.c 2003-10-28 10:52:08.000000000 +0000 > @@ -586,6 +586,9 @@ register struct parse *p; > register cset *cs = allocset(p); > register int invert = 0; > > + if (cs == NULL) > + return; > + > /* Dept of Truly Sickening Special-Case Kludges */ > if (p->next + 5 < p->end && strncmp(p->next, "[:<:]]", 6) == 0) { > EMIT(OBOW, 0); This change to p_bracket seems OK. > @@ -1015,25 +1018,49 @@ register struct parse *p; > nbytes = nc / CHAR_BIT * css; > if (p->g->sets == NULL) > p->g->sets = (cset *)malloc(nc * sizeof(cset)); > - else > - p->g->sets = (cset *)realloc((char *)p->g->sets, > - nc * sizeof(cset)); > - if (p->g->setbits == NULL) > - p->g->setbits = (uch *)malloc(nbytes); > else { > - p->g->setbits = (uch *)realloc((char *)p->g->setbits, > + cset *pgss = (cset *)realloc((char *)p->g->sets, > + nc * sizeof(cset)); > + if(pgss == NULL) { > + free(p->g->sets); > + p->g->sets = NULL; > + } > + else > + p->g->sets = pgss; > + } > + > + if (p->g->sets == NULL) { > + if (p->g->setbits != NULL) { > + free(p->g->setbits); > + p->g->setbits = NULL; > + } > + } else { > + if (p->g->setbits == NULL) > + p->g->setbits = (uch *)malloc(nbytes); > + else { > + uch *pgsbs = (uch *)realloc((char *)p->g->setbits, > nbytes); > - /* xxx this isn't right if setbits is now NULL */ > - for (i = 0; i < no; i++) > - p->g->sets[i].ptr = p->g->setbits + css*(i/CHAR_BIT); > + if (pgsbs != NULL) { > + p->g->setbits = pgsbs; > + for (i = 0; i < no; i++) > + p->g->sets[i].ptr = p->g->setbits + css*(i/CHAR_BIT); > + } else { > + free(p->g->setbits); > + p->g->setbits = NULL; > + } > + } > + if (p->g->setbits == NULL) { > + free(p->g->sets); > + p->g->sets = NULL; > + } > } > if (p->g->sets != NULL && p->g->setbits != NULL) > (void) memset((char *)p->g->setbits + (nbytes - css), > 0, css); > else { > - no = 0; > SETERROR(REG_ESPACE); > /* caller's responsibility not to do set ops */ > + return NULL; > } > } This change to allocset seems OK. > @@ -1161,8 +1188,14 @@ register char *cp; > cs->smultis += strlen(cp) + 1; > if (cs->multis == NULL) > cs->multis = malloc(cs->smultis); > - else > - cs->multis = realloc(cs->multis, cs->smultis); > + else { > + char *csm = realloc(cs->multis, cs->smultis); > + if (csm == NULL) { > + free(cs->multis); > + cs->multis = NULL; > + } else > + cs->multis = csm; > + } > if (cs->multis == NULL) { > SETERROR(REG_ESPACE); > return; This change to mcadd seems OK. What about fixing enlarge, mcsub and stripsnug? enlarge and mcsub may not need fixing, since they will assert, if the memory block could not be realloced. But perhaps that code should be fixed in case we build it with NDEBUG defined. (See "info libc alpha assert".) stripsnug needs fixing, because it could leak memory. As it stands, I don't this patch can go in. It needs testing, to check that the realloc fixes don't: (a) break any regexp functionality; (b) expose other bugs; (c) don't break any assumptions the code makes. I read the code and I think the patch is OK with regard to (c). But I'm not sure about (a) and (b). Thanks, bye, Rich =] -- Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ] "You can't evaluate a man by logic alone." -- McCoy, "I, Mudd", Star Trek