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
