[OPEN-ILS-DEV] PATCH: srfsh.c (more on shelling out)

[Mike Rylander]

On 7/13/07, Scott McKellar <mck9 at swbell.net> wrote:
>
> --- Mike Rylander <mrylander at gmail.com> wrote:
>
> > This is, in fact, the problem.  I've fixed it by using
> > calloc()/free()
> > instead of using an array.  calloc() has the extra benefit of zeroing
> > the memory, so the first memset() is no longer needed.  I left the
> > NULLification of the last + 1 pointer in place for systems where NULL
> > != all-0-bits.
>
> While your version of the fix should work, I don't know why you
> consider it an advantage to zero out memory, when every single zeroed
> byte will be either immediately overwritten or forever ignored.  The
> first memset() was never needed, or at best was overkill for
> nullifying a single pointer.

The advantage of (actually) zeroing out the memory is simply the
prinicpal of least surprise.  It was the intent before, so I corrected
the implementation and left the effect in place.  Upon further
reflection it may prove worthwhile to use another implementation to
avoid the calloc zeroing overhead, such as in the case of a script
which sends many small requests very quickly.  I'll test that ...

BTW, I've attempted to find evidence of a platform where NULL is not
all-0-bits and failed, though this only exposes my inability to tell
google what I want to find.

UPDATE: After testing the speed of calloc() -- 10ms total to allocate
1000 arrays of 4096 pointers on my machine; test cribbed from
http://www.scit.wlv.ac.uk/~jphb/spos/notes/alloc.html attached -- I'm
not too concerned about the speed.  Still, if a patch showed up to
revert the calloc change I certainly won't veto it.

>
> I would be inclined to leave the pointers in a declared array,
> possibly static, or even at file scope.  But if you'd rather
> allocate it and free it every time, that will work too.

While we don't have any logically nested calls to parse_request()
today, I don't see any reason to disallow it with a file-scoped array.

--miker