Re: src/kwset.c (kwsincr, kwsprep): obstack_alloc()-related fixes

[Julian Foad]

Charles Levert wrote:
> Proposed patch; I'll write a ChangeLog entry if
> it's accepted.

I'd much rather you wrote one now, so that I could more quickly understand the 
purpose and context of this change.

> The added call to obstack_free() will pop the top
> allocation off the obstack, which just succeeded
> before the last one failed.  It's just to keep
> things clean.
>
>
>
> --- src/kwset.c 2005-07-04 01:14:37 -0400
> +++ src/kwset.c 2005-07-04 09:39:09 -0400
> @@ -176,7 +176,10 @@ kwsincr (kwset_t kws, char const *text, 
>  	  link->trie = (struct trie *) obstack_alloc(&kwset->obstack,
>                                                      sizeof (struct trie));
>           if (!link->trie)
> -           return _("memory exhausted");
> +           {
> +             obstack_free(&kwset->obstack, link);
> +             return _("memory exhausted");
> +           }

OK, you've explained that part.

>           link->trie->accepting = 0;
>           link->trie->links = NULL;
>           link->trie->parent = trie;
> @@ -397,6 +400,8 @@ kwsprep (kwset_t kws)
>  
>        /* Looking for just one string.  Extract it from the trie. */
>        kwset->target = obstack_alloc(&kwset->obstack, kwset->mind);
> +      if (!kwset->target)
> +       return _("memory exhausted");

I know this change is sort of obvious - it's much nicer to throw an error 
message than a segmentation fault - but are you doing this because this is the 
only place left in the code where we weren't checking for failure, or because 
somebody has encountered this particular condition in practice but there may be 
other unchecked places still, or what?

This particular change is simple enough that I don't really care - I'd be happy 
for you to just commit this without much of an explanation - but in general I 
would like to know the reason for each change that we make.

- Julian the fussy.