Hello Stanislas, thanks for the review
On 7/27/24 00:48, Stanislav Fomichev wrote:
On 07/25, Alexis Lothoré (eBPF Foundation) wrote:
[...]
- if (should_fail)
ASSERT_ERR(ret, "mknod");- else
ASSERT_OK(ret, "mknod");Optional: might be easier to use something like expected_ret instead of should_fail and then do:
ASSERT_EQ(ret, expected_ret)
Yes, you are right. I initially went with a version relying on system() to perform the mknods/dd calls, which could return different errors codes so I used this should_fail. But while debugging some issues in CI with this series, I realized that the needed commands are basic enough to be replaced with direct library calls and I forgot to update this part, which can now assert an exact return value. I will update this accordingly.
I see this part being copy-pasted in a bunch of places below.
- unlink(path);
+}
+static void test_read(const char *path, int should_fail) +{
- char buf[TEST_BUFFER_SIZE];
- int ret, fd;
- fd = open(path, O_RDONLY);
- /* A bare open on unauthorized device should fail */
- if (should_fail) {
ASSERT_ERR(fd, "open file for read");[..]
if (fd)close(fd);nit: should this be 'if (fd >= 0)'? I'm assuming the intention is to avoid close(-1)?
Right as well, I'll fix it (here and below) in v2
Thanks,
Alexis