[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] fixes for apr_vformatter and apr_snprintf

From: Nuutti Kotivuori <naked_at_iki.fi>
Date: 2002-07-11 09:17:06 CEST

Jim Jagielski wrote:
> Nuutti Kotivuori wrote:
>> Jim Jagielski wrote:
>>> So if truncated, what is returned *must* be >= the length passed
>>> in.
>>
>> apr_snprintf is not snprintf.
>
> Agreed, but we want some of the same characteristics. Returning
> length upon truncation is one of them.
>
>> * In no event does apr_snprintf return a negative number. It's not
>> possible to distinguish between an output which was truncated, and
>> an output which exactly filled the buffer.
>>
>> If this comment is changed, I can fix try to fix these functions to
>> behave as expected again.
>
> Seems to me that this is the way apr_snprintf() has evolved to be. I
> can't recall the exact reasons why this happened, but I'm sure that
> the cvs logs provide the detail. Certainly, ap_snprintf() (from
> which apr_snprintf was derived) did not start this way, but was
> instead added in to provide an ANSI snprintf() implementation. But
> it has since been changed to not be a drop-in replacement of
> snprint() with, as you say, documented differences (ala cpystrn())

Jim Jagielski wrote:
> I'm guessing that if we *do* need to determine the diff between
> truncation and "just fits" we'd need to change the below
>
> if (sp >= bep) {
> if (flush_func(vbuff))
> return -1;
> }
> return cc;
>
> to
>
> if (sp > bep) {
> if (flush_func(vbuff))
> return -1;
> }
> return cc;
>
> since we have a *sp++ to deal with in INS_CHAR (as we want to be
> defensive in the coding). But again, the deal is whether that
> behavior is a documented feature or bug.

Jim Jagielski wrote:
> Looks like PR9932 explains why we have the current behavior... Not
> sure if the change from (sp >= bep) -> (sp > bep) makes sense in
> this case. We most likely need to flush when we are *at* bep, but
> I'm not really sure.

OK, I went around the CVS and I went to see the bug report.

And... I would consider the bugfix a bit of a hack. The problem being
solved there was that when outputting an empty string might cause
apr_psprintf to not handle pools correctly. And sure enough, when
peeking around the code for that, it is _nowhere_ making sure it can
_fit_ it's nul-byte in the pool - it's just blindly assuming the flush
handler to allocate any needed space. In fact:

    ps.vbuff.curpos = ps.node->first_avail;
    ps.vbuff.endpos = ps.node->endp - 1;

If first_avail == endp, then vbuff.endpos is actually smaller than
curpos. And it's only defensive programming that hides this problem
(using >= instead of ==).

IMO, the correct fix would be to make apr_psprintf behave like it
should - make sure there's room for it's nul-byte, instead of
depending on vformatter calling the flush.

***

But back to the subject at hand.

I'm a bit uncertain _how_ am I supposed to be using apr_snprintf so
code doesn't break the next time it's behaviour changes? I'm a bit on
a foul mood here, sorry, but this was supposed to be just a minor
checkup to see if the return value included the nul-byte or not.

So, if I want the length, without including the nul-byte - I need to
compare the return value against the buffer length I passed in, and
substract one if they are equal? I'd like to _depend_ on something
working this way, if I do it this way.

Thanks for your time.

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 11 09:17:54 2002

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.