Re: CVS update: subversion/subversion/libsvn_fs id.c

Originally, you could pass NULL to get a malloc. Jim had written that
if/else just like Ben did, and I went and "simplified" it. Then APR had a
mindset change and said "NULL pools ar Bad" and removed that capability. So
now we segfault if we apss NULL.

So the question becomes: wtf was NULL passed? I'm wary of anything that is
using malloc rather than a pool. IMO, we should always use a pool and never
fall back to malloc. Consider: who is calling free() on that thing?

Cheers,
-g

p.s. note the comment must be changed

On Tue, Feb 27, 2001 at 05:52:59PM -0600, Ben Collins-Sussman wrote:
>
> Uhhhhh, nope. apr_pcalloc was segfaulting on both me and Mike
> (fs-test #5). Adding the malloc fixed this.
>
> Maybe it's time to debug apr. :)
>
> Jim Blandy <jimb@zwingli.cygnus.com> writes:
>
> > This is unnecessary. apr_pcalloc behaves this way automatically
> > whenever its pool argument is zero.
> >
> > sussman@tigris.org writes:
> >
> > >
> > > User: sussman
> > > Date: 01/02/27 14:58:08
> > >
> > > Modified: subversion/libsvn_fs id.c
> > > Log:
> > > (svn_fs_parse_id): bugfix: make this func use malloc if no pool is
> > > given, as described in its documentation.
> > >
> > > Revision Changes Path
> > > 1.16 +8 -1 subversion/subversion/libsvn_fs/id.c
> > >
> > > Index: id.c
> > > ===================================================================
> > > RCS file: /cvs/subversion/subversion/libsvn_fs/id.c,v
> > > retrieving revision 1.15
> > > retrieving revision 1.16
> > > diff -u -r1.15 -r1.16
> > > --- id.c 2001/02/12 00:26:14 1.15
> > > +++ id.c 2001/02/27 22:58:08 1.16
> > > @@ -191,7 +191,14 @@
> > >
> > > /* Allocate the ID array. Note that if pool is zero, apr_palloc
> > > just calls malloc, which meets our promised interface. */
> > > - id = apr_palloc (pool, sizeof (*id) * (id_len + 1));
> > > + if (pool)
> > > + id = apr_palloc (pool, sizeof (*id) * (id_len + 1));
> > > + else
> > > + {
> > > + id = malloc (sizeof (*id) * (id_len + 1));
> > > + if (! id)
> > > + abort(); /* couldn't malloc */
> > > + }
> > >
> > > {
> > > int i = 0;
> > >
> > >
> > >
> > >

-- 
Greg Stein, http://www.lyra.org/