Fix the file-descriptor resource leakage as reported by Coverty

[aniketp]

Summary

Coverty reported leakage in resources in some of the audit tests which involved opening a
file and not clcosing it. This revision fixes that issue.

Also, some more ATF_REQUIRE statements to restrict errors.

NOTE: Earlier version had this snippet

utils.c:199 close(fd[0].fd);

returns -1 since the pipestream has already been closed one line above. So removed it

Test Plan

A part of report can be found here.

[asomers]

tests/sys/audit/file-attribute-access.c

63 ↗

(On Diff #43669)

A comment that simply repeats what the code does isn't a useful comment (except in assembly programming, where the code is filled with stuff like VPCLMULQDQ). You should delete these comments, or replace them with something like "teardown".

155 ↗

(On Diff #43669)

It's usually a bad idea to put assertions in teardown code, because a failed assertion can prevent the rest of the teardown from running. In this case it doesn't matter, because there's only one teardown statement. Still, it would be conventional to omit the ATF_REQUIRE_EQ.

tests/sys/audit/open.c

63 ↗

(On Diff #43669)

Mixing whitespace changes with functional changes is hard to review. Could you please revert the whitespace changes for now?

[aniketp]

Remove ATF_REQUIRE_EQ assertion from close() statements (and the comments)

[aniketp]

tests/sys/audit/open.c

79 ↗

(On Diff #43686)

@asomers , formatting the backslashes to be at the limit of 80 character line (which looks better) took quite a bit of hard work. Undoing them would be really cumbersome. Can you please review in the current state?

I've only added these lines in open.c

55: static int filedesc;   [Global]
79: close(filedesc);
119: close(filedesc);
120: close(filedesc2);

And modified the respective open() statements to store the return value in filedesc.
Rest of the code was already reviewed by you in D15657.

[asomers]

tests/sys/audit/open.c

79 ↗

(On Diff #43686)

Could you revert everything else then and submit a style-only change? It's not just about ease-of-review. It's also important for future programmers who try to figure out what this change did and why it did it. And yourself is included among those future programmers; a year from now you won't be able to remember what you did.

[aniketp]

Remove backslash formatting from open(2)'s test program macro
[Unrelated to this revision]

• Also add some ATF_REQUIRE assertions in utils.c. As their successful execution is important
• Correct includes' order in utils.c

[asomers]

tests/sys/audit/open.c

119 ↗

(On Diff #43690)

It doesn't matter in this case, but in general it's a good idea to release resources in the LIFO order as opposed to FIFO order. That way you never have an invalid combination of resources open.

[aniketp]

Invert the closing of file descriptors for open and openat following LIFO rule