Hacker News Reader

Comment by mbreese | Hacker News Reader

Best line of the post:

> And as it is with these types of bugs, finding the root cause is not trivial, but the fix almost always is.

This is actually the best case scenario: putting in some effort to find the root cause, having maybe a bit of luck and actually finding it, and then making a minimal change to fix it (hopefully) once and for all. Much better than the typical back-and-forth trial-and-error "I changed something, see if it works now" - which I suspect are much more common in the real world (especially if the reporter is a different person than the one fixing it), but are nothing worth blogging about...

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.

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.

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!

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/0ba8d616d1447...)

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.

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

Getting paid not to turn a screw but to know which screw to turn.

Reply on news.ycombinator.com