> I think there are three different things that need to be addressed: > > * Underspecified comments.
I agree. My patch added comments, the next one with actual fixes adds more. I just didn't want to invest even more time on them while the code is its current shape. > * The function names and argument names are badly chosen IMO, because even > granted a convention such as the above, it's not very obvious what roles > "l1" and "l2" play. I'm not exactly sure what would be better, but if you > used names like "ofseg" and "otherseg" you'd at least be trying. I'd go > with an asymmetrical function name too, to make miswriting of calls less > likely. Good idea. I haven't though about that, but now such names makes more sense to me. I will prepare another patch to improve the naming and comments to be applied on top of my current patches. > * And lastly, are we sure there aren't actual *bugs* here? I'd initially > supposed that lseg_closept_lseg acted as Emre says above, but reading the > code makes me think it's the other way around. Its first two potential > assignments to *result are definitely assigning points on l2 not l1. Yes, it is wrong beyond understanding. The committed patch intended to keep answers as they were. The next one actually fixes those.
