In D124244#3493307, @martong wrote:

The patch adds two new checkers.

I don't see any technical dependencies between the two, so, please split it into two independent patches.

fix clang format

Thanks for the review @martong

@balazske Thanks for the comments!

@NoQ can you please have another look at this? I think it will be a useful checker.

Gentle ping

@NoQ, thanks for the comments.

Removed code repetition from the tests.

Attaching results of CodeChecker run on some projects.

Fixed docs.

Please suggest which package to use for the checker.
CERT rules are ENV, however, it deals with non-ENV functions as well.

In D79358#2019755, @Szelethus wrote:

Adding @martong, because I fear that this is colliding with StdLibraryFunctionsChecker. The warnings added here seem to be, in essence, identical to D73898.

In D71433#1880436, @Szelethus wrote:

I think for an alpha checker this is ready to land if you're ready -- do you have commit access or need assistance?

• Removed dead code.

In D71433#1863638, @Charusso wrote:

In D71433#1808316, @Charusso wrote:

In D71433#1784238, @NoQ wrote:

Currently the check may warn on non-bugs of the following kind:

void foo() {
  char env[] = "NAME=value";
  putenv(env);
  doStuff();
  putenv("NAME=anothervalue");
}

That could be the next round as a follow-up patch as the next semester starts in February [...]

Well, the next semester is about to start. Could you implement that request please?

Addressed new inline comments.

• Removed extra test

In D71433#1784238, @NoQ wrote:

Thanks! This looks like a simple and efficient check. I have one overall suggestion.

Currently the check may warn on non-bugs of the following kind:

void foo() {
  char env[] = "NAME=value";
  putenv(env);
  doStuff();
  putenv("NAME=anothervalue");
}

I.e., it's obviously harmless if the local variable pointer is removed from the environment before it goes out of scope. Can we instead warn when the *last* putenv() on the execution path through the current stack frame is a local variable (that goes out of scope at the end of the stack frame)?

That'd allow the checker to be enabled by default, which will give a lot more users access to it. Otherwise we'll have to treat it as an opt-in check and users will only enable it when they know about it, which is much less usefulness.

Addressed most of the inline comments.

changes after review.