15
\$\begingroup\$

As part of our check-list for questions on CodeReview, we require that code is working to the best of the askers knowledge.

Does code with a known memory leak qualify is broken per this rule?

On the one hand, the code most likely does everything the asker wants, and from the user may never notice unless the code which creates the leak is executed so many times that the program runs out of memory.

On the other hand, there's a memory leak!

\$\endgroup\$
14
\$\begingroup\$

Code with known memory leaks (or other resource leaks, such as file descriptors) might be working or broken depending on the context.

If it is code that is meant to be in a long-running process, then any memory leak could be considered a fatal flaw that renders the code broken, and therefore the question off-topic.

On the other hand, a short-lived program might suffer no apparent ill effects from a memory leak. In that case, the question could be considered on-topic, and the sloppiness would be fair material to be addressed in an answer.

\$\endgroup\$
  • 2
    \$\begingroup\$ While I agree, it might be difficult for us to draw that line. \$\endgroup\$ – RubberDuck Aug 21 '14 at 14:12
  • 10
    \$\begingroup\$ @RubberDuck I know, gray areas are annoying. If in doubt, the question should stay open so that the leaks can be addressed in answers. \$\endgroup\$ – 200_success Aug 21 '14 at 15:41
10
\$\begingroup\$

If the code executes, it should be free game to review. Depending on the use-case, you may never notice that there is a problem during normal operation.

However, pointing out the leaks and fixes for them is quite valid as part of a review.

For example, I commonly see questions with code in need of using statements. These could be considered memory leaks, especially when unmanaged resources are involved.

It is better to be welcoming and inclusive than shun users over a topic that is not universally understood, especially considering our perma-beta status.

\$\endgroup\$
  • \$\begingroup\$ This question is about known memory leaks--not unknown memory leaks. \$\endgroup\$ – nhgrif Aug 20 '14 at 23:32
  • \$\begingroup\$ @nhgrif I may have misinterpreted "known leak" as "someone (i.e., the reader) knows there is a memory leak" because it's late in the day and I don't recall seeing many (any?) examples of the former. Do you have any you could link? \$\endgroup\$ – Dan Lyons Aug 20 '14 at 23:51
  • \$\begingroup\$ This question is about when the asker knows there is a memory leak as he is posting the question. \$\endgroup\$ – nhgrif Aug 20 '14 at 23:55
  • 2
    \$\begingroup\$ Fair enough. If the point of the question is to get others to find and resolve the memory leak, then by all means, it should be remanded to SO. \$\endgroup\$ – Dan Lyons Aug 21 '14 at 0:03
  • 1
    \$\begingroup\$ @DanLyons this question about finding a memory leak would be one example. I agree that if you find a memory leak, but the user never asked about it, then the question would still be on topic \$\endgroup\$ – tim Aug 21 '14 at 9:33
  • 1
    \$\begingroup\$ For the record, I agree with that sentiment as well. We only ask the code works to the best of the asker's knowledge. \$\endgroup\$ – nhgrif Aug 21 '14 at 11:22
5
\$\begingroup\$

Code with known memory leaks should be considered categorically off-topic on Code Review. To me, this is considered broken code.

I might release code that looks a little sloppy, or runs a little efficiently, but releasing an application with a known memory leak is just as unacceptable as releasing an application with bugs that prevent usability.

\$\endgroup\$
  • 5
    \$\begingroup\$ If I allocate a new char[] in a function in C++ and forget to delete it, will my question get closed as broken, even although it works on my machine? \$\endgroup\$ – Pimgd Sep 30 '14 at 17:56
  • \$\begingroup\$ This is rather one-sided. There may be situations where not freeing aquired resources is an optimization. E.g. in an embedded context, where the program will only terminate when the power is turned off. Or if we need to keep hold of those resources until the program terminates and know the OS will free them at that time. \$\endgroup\$ – D Drmmr Jan 28 at 19:48

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .