3 ms·

Well, except that his fix can't easily be pushed back into the source tree because he doesn't know what other software is only working because of the original f

Well, except that his fix can't easily be pushed back into the source tree because he doesn't know what other software is only working because of the original fix. Presumably it must have been needed by at least one other person, probably also after a crazy long debugging session, or it wouldn't have been there at all. As this code is in a public repo for the default library of a well-used language, it's pretty much a given that fixing this by just removing those lines will cause a regression for someone.

pixl97 2y ago
Heh, which is why you document the code when adding hacks like this in the first place... >#Adding this line to prevent hangs on XXX terminal Or whatever.
- OJFord 2y ago
  There was in this case, just above the offending lines, including this gem: > [...] This does NOT appoear [sic] to happen with TCP sockets, but this doesn't hurt the TCP side in any case. Oops!
gjm11 2y ago
Unless I'm misunderstanding the post, it's the author's own code. (Here's the relevant change, removing those four lines of code: https://github.com/spc476/lua-conmanorg/commit/0ba8d616d14477603b33fd8f519d9a7b445235bd https://github.com/spc476/lua-conmanorg/commit/0ba8d616d1447...)
- ralferoo 2y ago
  Oh good spot. I'd misread the repo name as lua-common, I guess my brain "autocorrected" it. The way the article was written, I didn't at all get the impression that it was his code, just that he discovered it and tried hacking it out: > This bit of code wasn't in the TLS implementation, and as the comment says, it “shouldn't” hurt the TCP path, but hey, it was only needed for local (or Unix) domain sockets to begin with, so let me try removing that bit of code and run a test. But yeah, if it's his own project, it's far easier to keep track of and watch out for any bug reports that might result from this change.
stanleydrew 2y ago
The fix doesn't require removing code though as far as I can tell. Could just add an if statement.

pixl97 2y ago

Heh, which is why you document the code when adding hacks like this in the first place... >#Adding this line to prevent hangs on XXX terminal Or whatever.

OJFord 2y ago
There was in this case, just above the offending lines, including this gem: > [...] This does NOT appoear [sic] to happen with TCP sockets, but this doesn't hurt the TCP side in any case. Oops!

gjm11 2y ago

Unless I'm misunderstanding the post, it's the author's own code. (Here's the relevant change, removing those four lines of code: https://github.com/spc476/lua-conmanorg/commit/0ba8d616d14477603b33fd8f519d9a7b445235bd https://github.com/spc476/lua-conmanorg/commit/0ba8d616d1447...)

ralferoo 2y ago
Oh good spot. I'd misread the repo name as lua-common, I guess my brain "autocorrected" it. The way the article was written, I didn't at all get the impression that it was his code, just that he discovered it and tried hacking it out: > This bit of code wasn't in the TLS implementation, and as the comment says, it “shouldn't” hurt the TCP path, but hey, it was only needed for local (or Unix) domain sockets to begin with, so let me try removing that bit of code and run a test. But yeah, if it's his own project, it's far easier to keep track of and watch out for any bug reports that might result from this change.

stanleydrew 2y ago

The fix doesn't require removing code though as far as I can tell. Could just add an if statement.