On Thursday 27 October 2011, Jim Jagielski wrote:
> On Oct 26, 2011, at 3:46 PM, Stefan Fritsch wrote:
> > On Wednesday 26 October 2011, Jim Jagielski wrote:
> >> On Oct 25, 2011, at 6:29 PM, s...@apache.org wrote:
> >>> + if (len > maxlen && maxlen > 0)
> >>> + return APR_ENOMEM;
> >>> +
> >>>
> >>> if (!vb) {
> >>>
> >>> - dest = dst = apr_pcalloc(p, len + 1);
> >>> + *result = dst = apr_pcalloc(p, len + 1);
> >>
> >> if len == maxlen and == APR_SIZE_MAX then doesn't
> >> the len+1 blow us up?
> >
> > APR_SIZE_MAX means no limit, i.e. it will blow up when there is
> > no memory left, which will be well before APR_SIZE_MAX. I guess
> > I should have used 0, that would have been clearer.
>
> I thought that
>
> #define APR_SIZE_MAX (~((apr_size_t)0))
>
> returned the one's complement of 0, which would basically
> be the largest value possible for (apr_size_t)0, and that
> adding 1 to it would overflow… right? Or am I still jet-lagged
> (which is quite possible).
That's correct, it would crash if len == APR_SIZE_MAX. But my point
was that it would also crash for len == APR_SIZE_MAX-1000, because by
definition, the machine cannot have that much free mem and apr_pcalloc
would call abort(). So in both cases, the amount of free memory is the
limit. But aborting with out-of-mem is more correct than simply
segfaulting, therefore I have fixed it.
> The main question is whether or not maxlen accounts for
> the NUL or not; we seem inconsistent in that regard with
> this patch.
True. I think this is also fixed with r1189985.
