lkml.org 
[lkml]   [2021]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 bpf-next 2/2] selftests: bpf: Add a new test for bare tracepoints
Hi Yonghong

On 01/18/21 09:48, Yonghong Song wrote:
> The original patch code:
>
> +static int trigger_module_test_write(int write_sz)
> +{
> + int fd, err;
> + char *buf = malloc(write_sz);
> +
> + if (!buf)
> + return -ENOMEM;
> +
> + memset(buf, 'a', write_sz);
> + buf[write_sz-1] = '\0';
> +
> + fd = open("/sys/kernel/bpf_testmod", O_WRONLY);
> + err = -errno;
> + if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
> + goto out;
> +
> + write(fd, buf, write_sz);
> + close(fd);
> +out:
> + free(buf);
> +
> + return 0;
> +}
>
> Even for "fd < 0" case, it "goto out" and "return 0". We should return
> error code here instead of 0.
>
> Second, "err = -errno" is set before checking fd < 0. If fd >= 0, err might
> inherit an postive errno from previous failure.
> In trigger_module_test_write(), it is okay since the err is only used
> when fd < 0:
> err = -errno;
> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
> return err;
>
> My above rewrite intends to use "err" during final "return" statement,
> so I put assignment of "err = -errno" inside the CHECK branch.
> But there are different ways to implement this properly.

Okay I see now. Sorry I missed your point initially. I will fix and send v3.

Thanks

--
Qais Yousef

\
 
 \ /
  Last update: 2021-01-19 14:11    [W:0.095 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site