search

coin-or / Clp

COIN-OR Linear Programming Solver
Other
392 stars 82 forks source link

fix memory leak in ctor of ClpNetworkMatrix #175 #255

Closed tobiass-sdl closed 1 year ago

tobiass-sdl commented 1 year ago

This fixes https://github.com/coin-or/Clp/issues/175 and removes 2 redundant ';'.

Note that my employer insists on that copyright statement no matter how trivial the change is. Please let me know if that copyright claim is an issue. I'm aware that if this is done for every change the file headers will become grow quite a bit.

jhmgoossens commented 1 year ago

Nice one-line fix IMO (a missing delete[] which;), but adding an extra copyright to the top of the file for this? Any thoughts? @svigerske @tkralphs

tkralphs commented 1 year ago

Yeah, adding a copyright notice for a one line fix is definitely not justified. I'm sure this is just some corporate legal requirement that all open-source contributions come with this kind of attribution, but I would not accept this PR as it is.

tobiass-sdl commented 1 year ago

We had internal discussions about that copyright claim before submitting the PR and quite a few software developer stated what they would do if someone would submit such a PR/copyright claim for their open source project. Nevertheless our license expert reviewed the proposed PR (i.e. he knew that we are talking about a single delete statement) and insisted on that copyright claim stating that an OSS project moderator should not have any problems with that. I won't comment on that. Therefore do whatever you like with this PR. Personally I do understand if it is rejected due to that copyright claim.

svigerske commented 1 year ago

I think 54b5d0b0e works around this just fine. Thanks @tobiass-sdl for reporting and pushing for a fix.

tkralphs commented 1 year ago

Yes, that is cleaner, thanks @svigerske.