Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1501578 > unrolled thread

[PATCH 1/8] tools lib bpf: add error functions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/8] tools lib bpf: add error functions Eric Leblond <eric@regit.org> - 2016-10-17 00:10 +0200
    Re: [PATCH 1/8] tools lib bpf: add error functions "Wangnan (F)" <wangnan0@huawei.com> - 2016-10-17 04:00 +0200
    Re: [PATCH 1/8] tools lib bpf: add error functions Joe Stringer <joe@ovn.org> - 2016-10-19 01:00 +0200
      Re: [PATCH 1/8] tools lib bpf: add error functions "Wangnan (F)" <wangnan0@huawei.com> - 2016-10-19 04:00 +0200

#1501578 — [PATCH 1/8] tools lib bpf: add error functions

The include of err.h is not explicitely needed in exported
functions and it was causing include conflict with some existing
code due to redefining some macros.

To fix this, let's have error handling functions provided by the
library. Furthermore this will allow user to have an homogeneous
API.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/libbpf.c | 11 +++++++++++
 tools/lib/bpf/libbpf.h |  4 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b699aea..90932f1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -31,6 +31,7 @@
 #include <linux/kernel.h>
 #include <linux/bpf.h>
 #include <linux/list.h>
+#include <linux/err.h>
 #include <libelf.h>
 #include <gelf.h>
 
@@ -1447,3 +1448,13 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
 	}
 	return NULL;
 }
+
+bool bpf__is_error(const void *ptr)
+{
+	return IS_ERR(ptr);
+}
+
+long bpf__get_error(const void *ptr)
+{
+	return PTR_ERR(ptr);
+}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index dd7a513..e40c8d3 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -23,7 +23,6 @@
 
 #include <stdio.h>
 #include <stdbool.h>
-#include <linux/err.h>
 
 enum libbpf_errno {
 	__LIBBPF_ERRNO__START = 4000,
@@ -211,4 +210,7 @@ int bpf_map__set_priv(struct bpf_map *map, void *priv,
 		      bpf_map_clear_priv_t clear_priv);
 void *bpf_map__priv(struct bpf_map *map);
 
+bool bpf__is_error(const void *ptr);
+long bpf__get_error(const void *ptr);
+
 #endif
-- 
2.9.3

[toc] | [next] | [standalone]

#1501591

On 2016/10/17 5:18, Eric Leblond wrote:
> The include of err.h is not explicitely needed in exported
> functions and it was causing include conflict with some existing
> code due to redefining some macros.
>
> To fix this, let's have error handling functions provided by the
> library. Furthermore this will allow user to have an homogeneous
> API.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
>   tools/lib/bpf/libbpf.c | 11 +++++++++++
>   tools/lib/bpf/libbpf.h |  4 +++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b699aea..90932f1 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -31,6 +31,7 @@
>   #include <linux/kernel.h>
>   #include <linux/bpf.h>
>   #include <linux/list.h>
> +#include <linux/err.h>
>   #include <libelf.h>
>   #include <gelf.h>
>   
> @@ -1447,3 +1448,13 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
>   	}
>   	return NULL;
>   }
> +
> +bool bpf__is_error(const void *ptr)

Please use libbpf_is_error(), like libbpf_set_print. We use '__' because 
we want
to use the OO concept. This utility is not OO.

> +{
> +	return IS_ERR(ptr);
> +}
> +
> +long bpf__get_error(const void *ptr)

Same, please call it libbpf_get_error().

Thank you.

[toc] | [prev] | [next] | [standalone]

#1503413

On 16 October 2016 at 14:18, Eric Leblond <eric@regit.org> wrote:
> The include of err.h is not explicitely needed in exported
> functions and it was causing include conflict with some existing
> code due to redefining some macros.
>
> To fix this, let's have error handling functions provided by the
> library. Furthermore this will allow user to have an homogeneous
> API.
>
> Signed-off-by: Eric Leblond <eric@regit.org>

Does it need to return the error like this or should we just fix up
the bpf_object__open() API to return errors in a simpler form?

There's already libbpf_set_print(...) for outputting errors, is it
reasonable to just change the library to return NULLs in error cases
instead?

[toc] | [prev] | [next] | [standalone]

#1503494

On 2016/10/19 6:52, Joe Stringer wrote:
> On 16 October 2016 at 14:18, Eric Leblond <eric@regit.org> wrote:
>> The include of err.h is not explicitely needed in exported
>> functions and it was causing include conflict with some existing
>> code due to redefining some macros.
>>
>> To fix this, let's have error handling functions provided by the
>> library. Furthermore this will allow user to have an homogeneous
>> API.
>>
>> Signed-off-by: Eric Leblond <eric@regit.org>
> Does it need to return the error like this or should we just fix up
> the bpf_object__open() API to return errors in a simpler form?
>
> There's already libbpf_set_print(...) for outputting errors, is it
> reasonable to just change the library to return NULLs in error cases
> instead?

Returning error code to caller so caller knows what happen.
Other subsystems in perf also do this.

Perf hides libbpf's error output (make it silent unless -v),
so it needs a way for receiving libbpf's error code.

I think this patch is good, decouple libbpf.h and kernel headers.

Thank you.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web