BPF Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf-next] libbpf: make destructors more robust by handling ERR_PTR(err) cases
@ 2020-07-29 23:21 Andrii Nakryiko
  2020-07-30  4:20 ` Song Liu
  2020-07-31  0:05 ` Daniel Borkmann
  0 siblings, 2 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2020-07-29 23:21 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu

Most of libbpf "constructors" on failure return ERR_PTR(err) result encoded as
a pointer. It's a common mistake to eventually pass such malformed pointers
into xxx__destroy()/xxx__free() "destructors". So instead of fixing up
clean up code in selftests and user programs, handle such error pointers in
destructors themselves. This works beautifully for NULL pointers passed to
destructors, so might as well just work for error pointers.

Suggested-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.c      | 4 ++--
 tools/lib/bpf/btf_dump.c | 2 +-
 tools/lib/bpf/libbpf.c   | 9 ++++-----
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index c9e760e120dc..ded5b29965f9 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -386,7 +386,7 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
 
 void btf__free(struct btf *btf)
 {
-	if (!btf)
+	if (IS_ERR_OR_NULL(btf))
 		return;
 
 	if (btf->fd >= 0)
@@ -1025,7 +1025,7 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
 
 void btf_ext__free(struct btf_ext *btf_ext)
 {
-	if (!btf_ext)
+	if (IS_ERR_OR_NULL(btf_ext))
 		return;
 	free(btf_ext->data);
 	free(btf_ext);
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index e1c344504cae..cf711168d34a 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -183,7 +183,7 @@ void btf_dump__free(struct btf_dump *d)
 {
 	int i, cnt;
 
-	if (!d)
+	if (IS_ERR_OR_NULL(d))
 		return;
 
 	free(d->type_states);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 54830d603fee..b9f11f854985 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6504,7 +6504,7 @@ void bpf_object__close(struct bpf_object *obj)
 {
 	size_t i;
 
-	if (!obj)
+	if (IS_ERR_OR_NULL(obj))
 		return;
 
 	if (obj->clear_priv)
@@ -7690,7 +7690,7 @@ int bpf_link__destroy(struct bpf_link *link)
 {
 	int err = 0;
 
-	if (!link)
+	if (IS_ERR_OR_NULL(link))
 		return 0;
 
 	if (!link->disconnected && link->detach)
@@ -8502,7 +8502,7 @@ void perf_buffer__free(struct perf_buffer *pb)
 {
 	int i;
 
-	if (!pb)
+	if (IS_ERR_OR_NULL(pb))
 		return;
 	if (pb->cpu_bufs) {
 		for (i = 0; i < pb->cpu_cnt; i++) {
@@ -9379,8 +9379,7 @@ void bpf_object__detach_skeleton(struct bpf_object_skeleton *s)
 	for (i = 0; i < s->prog_cnt; i++) {
 		struct bpf_link **link = s->progs[i].link;
 
-		if (!IS_ERR_OR_NULL(*link))
-			bpf_link__destroy(*link);
+		bpf_link__destroy(*link);
 		*link = NULL;
 	}
 }
-- 
2.24.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] libbpf: make destructors more robust by handling ERR_PTR(err) cases
  2020-07-29 23:21 [PATCH bpf-next] libbpf: make destructors more robust by handling ERR_PTR(err) cases Andrii Nakryiko
@ 2020-07-30  4:20 ` Song Liu
  2020-07-31  0:05 ` Daniel Borkmann
  1 sibling, 0 replies; 3+ messages in thread
From: Song Liu @ 2020-07-30  4:20 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Kernel Team, Song Liu

On Wed, Jul 29, 2020 at 4:22 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Most of libbpf "constructors" on failure return ERR_PTR(err) result encoded as
> a pointer. It's a common mistake to eventually pass such malformed pointers
> into xxx__destroy()/xxx__free() "destructors". So instead of fixing up
> clean up code in selftests and user programs, handle such error pointers in
> destructors themselves. This works beautifully for NULL pointers passed to
> destructors, so might as well just work for error pointers.
>
> Suggested-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] libbpf: make destructors more robust by handling ERR_PTR(err) cases
  2020-07-29 23:21 [PATCH bpf-next] libbpf: make destructors more robust by handling ERR_PTR(err) cases Andrii Nakryiko
  2020-07-30  4:20 ` Song Liu
@ 2020-07-31  0:05 ` Daniel Borkmann
  1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2020-07-31  0:05 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast; +Cc: andrii.nakryiko, kernel-team, Song Liu

On 7/30/20 1:21 AM, Andrii Nakryiko wrote:
> Most of libbpf "constructors" on failure return ERR_PTR(err) result encoded as
> a pointer. It's a common mistake to eventually pass such malformed pointers
> into xxx__destroy()/xxx__free() "destructors". So instead of fixing up
> clean up code in selftests and user programs, handle such error pointers in
> destructors themselves. This works beautifully for NULL pointers passed to
> destructors, so might as well just work for error pointers.
> 
> Suggested-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Applied, thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 23:21 [PATCH bpf-next] libbpf: make destructors more robust by handling ERR_PTR(err) cases Andrii Nakryiko
2020-07-30  4:20 ` Song Liu
2020-07-31  0:05 ` Daniel Borkmann

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git