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. Regards, Joe
