Mail Archives: djgpp-workers/2003/11/09/17:41:38
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
- Raw text -