apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r1862071 - in /apr/apr/trunk: file_io/os2/dir.c file_io/unix/dir.c file_io/win32/dir.c include/apr_file_info.h test/testdir.c
Date Wed, 03 Jul 2019 18:22:45 GMT


On 07/03/2019 05:37 PM, Joe Orton wrote:
> On Wed, Jul 03, 2019 at 02:43:02PM +0300, Ivan Zhakov wrote:
>> They also make the existing old API unusable for many cases thus 
>> making a switch to the new API mandatory, even though it doesn't have 
>> to be so (see below).
>>
>> APR 1.x did not specify that the result of apr_dir_read() has to be allocated
>> in dir->pool:
>>
>>   https://apr.apache.org/docs/apr/1.7/group__apr__dir.html#ga3e4ee253e0c712160bee10bfb9c8e4a8
>>
>> This function does not accept a pool argument, and I think that it's natural
>> to assume limited lifetime in such case. This is what the current major APR
>> users (httpd, svn) do, and other users should probably be ready for that too,
>> since on Win32 the subsequent calls to apr_dir_read() already invalidate the
>> previous apr_finfo_t.
> 
> Are there many examples in the APR API where returned strings are not 
> either pool-allocated or constant (process lifetime)?  It seems unusual 
> and very surprising to me.
> 
> A hypothetical API consumer can have made either assumption:
> 
> a) of constant memory (true with limits on Win32, breaks for Unix), or
> b) of pool-allocated string lifetime (works for Unix, breaks for Win32)
> 
> neither is documented and both are broken by current implementations. It 
> is impossible to satisfy both so anybody can argue that fixing either 
> implementation to match the other is a regression.  
> 
> Doubling down on (a) over (b) seems strongly inferior to me, the API 
> cannot support this without introducing runtime overhead for all callers 
> (a new iterpool in the dir object), so I'd rather fix this properly with 
> a new API.
> 
> I'd be fine with leaving Win32 apr_dir_read() behaviour as-is for 1.x at 
> the same as introducing _pread() if desired - making the strdup only 
> happen for _pread would only be a minor tweak, not a whole new subpool.
> 
+1

Regards

RĂ¼diger



Mime
View raw message