[[ sorry for delayed response ]] On Fri, 7 Jan 2022 at 17:33, Yann Ylavic <ylavic....@gmail.com> wrote: > > Hi Ivan, > > On Fri, Jan 7, 2022 at 2:50 PM Ivan Zhakov <i...@visualsvn.com> wrote: > > > > This change does not compile on Windows in APR 1.7.x: > > [[[ > > file_io\win32\readwrite.c(325): error C2065: 'file': undeclared identifier > > file_io\win32\readwrite.c(325): error C2223: left of '->filehand' must > > point to struct/union > > I was missing backport of r1895178, does r1896808 compile now? > (Sorry, no Windows at hand..). Yes, it builds now. Thanks!
> > > > > I also have a high-level objection against backporting this change to > > APR 1.7.x: IMHO APR 1.7.x is a stable branch and I think that only > > regression fixes should be backported to the stable branch. r1896510 > > is a significant change and as far as I understand it's not a > > regression fix. So I think it would be better to revert r1896510 and > > release it as part of APR 2.0 (or 1.8.x). > > I think that most if not all of the changes to 1.7.x since 1.7.0 are > fixes for bugs that were there before 1.7 already, not regressions > introduced by 1.7.0. Agreed on the bugfix/regressions part. I have misunderstood that r1896510 is a bugfix, perhaps, due to its size, and was thinking that it adds new functionality. But even with that in mind, I still think that the size of the change might be just too large for it to be an appropriate fit for a patch release. Speaking of the change itself, I think that there might be an alternative to making the apr_file_t also handle sockets on Windows. It might be better to specifically change the pollset implementation so that on Windows it would add a socket and use it for wakeup, instead of using the socket disguised as a file. If this alternative approach sounds fine, I could try to implement it. -- Ivan Zhakov
