Remove legacy networking code, vol 2
[PATCH setup 1/2] NetIO: Remove unused url parsing code [PATCH setup 2/2] Improve file:// url handling netio.cc | 85 ++--- netio.h| 11 ++- nio-ie5.cc | 3 +-- 3 files changed, 21 insertions(+), 78 deletions(-)
Re: Remove legacy networking code
On 28/01/2018 21:47, Jon Turney wrote:
On 26/01/2018 16:56, SZAVAI Gyula wrote:
[PATCH setup 1/5] Remove direct(legacy) connection type
[PATCH setup 2/5] Rename IDC_NET_IE5 to IDC_NET_PRECONFIG
[PATCH setup 3/5] Remove NetIO_File
[PATCH setup 4/5] Add Proxy class
[PATCH setup 5/5] Remove NetIO_HTTP
Thanks, this is great.
I applied this patch series to setup, with a few tweaks to address my
comments.
Sorry about the delay and thanks again!
A few minor comments:
1/5:
It would be nice if you'd said in the patch commentary what will happen
to existing installations which are configured to use direct(legacy) -
they get converted to preconfig.
3/5:
This changes to wininet handling all file access (converting raw paths
into file:// format URLs)
I had some concerns [1] that this maybe changes the set of malformed
file:// URLs we handle (which is ok, although it would be nice to know
the details), and still doesn't handle correctly formed ones.
[1] https://cygwin.com/ml/cygwin-apps/2017-05/msg00117.html
4/5:
operator!= could probably use a comment along the lines of "it's only
meaningful to compare host:port if IDC_NET_PROXY"
It's unclear to me what Proxy::bypass() is doing. MSDN actually says
"Do not use an empty string", for unclear reasons.
We can also remove simpsock.{cc,h} and linking with winsock, after this
series.
Re: Remove legacy networking code
On 26/01/2018 16:56, SZAVAI Gyula wrote:
[PATCH setup 1/5] Remove direct(legacy) connection type
[PATCH setup 2/5] Rename IDC_NET_IE5 to IDC_NET_PRECONFIG
[PATCH setup 3/5] Remove NetIO_File
[PATCH setup 4/5] Add Proxy class
[PATCH setup 5/5] Remove NetIO_HTTP
Thanks, this is great.
A few minor comments:
1/5:
It would be nice if you'd said in the patch commentary what will happen
to existing installations which are configured to use direct(legacy) -
they get converted to preconfig.
3/5:
This changes to wininet handling all file access (converting raw paths
into file:// format URLs)
I had some concerns [1] that this maybe changes the set of malformed
file:// URLs we handle (which is ok, although it would be nice to know
the details), and still doesn't handle correctly formed ones.
[1] https://cygwin.com/ml/cygwin-apps/2017-05/msg00117.html
4/5:
operator!= could probably use a comment along the lines of "it's only
meaningful to compare host:port if IDC_NET_PROXY"
It's unclear to me what Proxy::bypass() is doing. MSDN actually says
"Do not use an empty string", for unclear reasons.
We can also remove simpsock.{cc,h} and linking with winsock, after this
series.
Remove legacy networking code
[PATCH setup 1/5] Remove direct(legacy) connection type [PATCH setup 2/5] Rename IDC_NET_IE5 to IDC_NET_PRECONFIG [PATCH setup 3/5] Remove NetIO_File [PATCH setup 4/5] Add Proxy class [PATCH setup 5/5] Remove NetIO_HTTP ConnectionSetting.cc | 11 ++--- Makefile.am | 6 --- net.cc | 17 +++ netio.cc | 42 - nio-file.cc | 64 -- nio-file.h | 31 - nio-ftp.cc | 179 nio-ftp.h| 41 - nio-http.cc | 208 nio-http.h | 39 nio-ie5.cc | 89 +--- nio-ie5.h| 2 +- res.rc | 4 +- resource.h | 3 +- 14 files changed, 96 insertions(+), 640 deletions(-)
