apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nuutti Kotivuori <na...@iki.fi>
Subject Re: [PATCH] fixes for apr_vformatter and apr_snprintf
Date Thu, 11 Jul 2002 07:17:06 GMT
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

Mime
View raw message