From murdoch at stats.uwo.ca  Fri Jan 25 16:48:08 2008
From: murdoch at stats.uwo.ca (Duncan Murdoch)
Date: Fri, 25 Jan 2008 10:48:08 -0500
Subject: [Rd] strsignif.c, util.c (PR#10635)
In-Reply-To: <20080125132515.AF172283462C@mail.pubhealth.ku.dk>
References: <20080125132515.AF172283462C@mail.pubhealth.ku.dk>
Message-ID: <479A04B8.7010205@stats.uwo.ca>

On 1/25/2008 8:25 AM, ripley at stats.ox.ac.uk wrote:
> On Fri, 25 Jan 2008, A.R.Runnalls at kent.ac.uk wrote:
> 
>> In R 2.6.1, a couple of places (discovered using valgrind) where the
>> requested size of string buffers fails to account correctly for the
>> trailing null byte:
>>
>> 1. In src/appl/strsignif.c, 'f0' and 'form' at l. 108-9 each need at
>> least 1 extra byte.
>>
>> 2. In src/main/util.c, 'out' at l. 1081 needs at least one extra byte.
>>
>> (Remember that the return value of strlen does not include the null byte.)
> 
> But it is subtler than that.  R_alloc contains the statement
> 
>  	s = allocVector(RAWSXP, size + 1);
> 
> and so does over-allocate by at least one (there is a rounding up to a 
> multiple of 8).  This is a historical anomaly (it used to allocate a 
> CHARSXP that allowed for the null byte), but one which trying to eliminate 
> caused too many crashes in package code.
> 
> I'd like to see the empirical evidence you have, as I have been unable to 
> trigger an overrun here.

That is not documented in Writing R Extensions or R Internals, so I 
think a change is needed, either to the docs or the calls.  I've already 
changed these calls.

I'd rather keep the docs as they are, because they give a sensible 
definition to the function.  If the implementation protects against 
sloppy usage that's okay, but I don't think we should take advantage of 
it, in case some future maintainer notices the inconsistency and removes it.

Duncan Murdoch



[Rd] strsignif.c, util.c (PR#10635)
Duncan Murdoch