| Message ID | 20180521065940.srb4rqwaohkn43i6@vm4 |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [1/1] tools/lib/libbpf.c: fix string format to allow build on arm32 | expand |
[ +Martin ] On 05/21/2018 08:59 AM, Sirio Balmelli wrote: > On arm32, 'cd tools/testing/selftests/bpf && make' fails with: > > libbpf.c:80:10: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘int64_t {aka long long int}’ [-Werror=format=] > (func)("libbpf: " fmt, ##__VA_ARGS__); \ > ^ > libbpf.c:83:30: note: in expansion of macro ‘__pr’ > #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__) > ^~~~ > libbpf.c:1072:3: note: in expansion of macro ‘pr_warning’ > pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", > > To fix, include 'inttypes.h' and change format directive to 'PRIi64'. > > Signed-off-by: Sirio Balmelli <sirio@b-ad.ch> Thanks for the fix! One minor comment below. > --- > tools/lib/bpf/libbpf.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 3dbe217bf23e..e2cc8f10c188 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -48,6 +48,8 @@ > #include "bpf.h" > #include "btf.h" > > +#include <inttypes.h> /* PRIi64 */ > + > #ifndef EM_BPF > #define EM_BPF 247 > #endif > @@ -1042,7 +1044,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > } > > if (def->key_size != key_size) { > - pr_warning("map:%s key_type:%s has BTF type_size:%ld != key_size:%u\n", > + pr_warning("map:%s key_type:%s has BTF type_size:%"PRIi64" != key_size:%u\n", > map->name, name, key_size, def->key_size); > return -EINVAL; > } > @@ -1069,7 +1071,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > } > > if (def->value_size != value_size) { > - pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", > + pr_warning("map:%s value_type:%s has BTF type_size:%"PRIi64" != value_size:%u\n", > map->name, name, value_size, def->value_size); I don't think we need the PRIi64 in here. Could you just change it to 'type_size:%u' and then cast the 'key_size' resp. 'value_size' to unsigned int? We know at this point that the type size value is positive anyway, so we only want to show the size mismatch so simple cast should suffice. Thanks! > return -EINVAL; > } >
On Wed, May 23, 2018 at 12:41:14PM +0200, Daniel Borkmann wrote: > [ +Martin ] > > On 05/21/2018 08:59 AM, Sirio Balmelli wrote: > > On arm32, 'cd tools/testing/selftests/bpf && make' fails with: > > > > libbpf.c:80:10: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘int64_t {aka long long int}’ [-Werror=format=] > > (func)("libbpf: " fmt, ##__VA_ARGS__); \ > > ^ > > libbpf.c:83:30: note: in expansion of macro ‘__pr’ > > #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__) > > ^~~~ > > libbpf.c:1072:3: note: in expansion of macro ‘pr_warning’ > > pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", > > > > To fix, include 'inttypes.h' and change format directive to 'PRIi64'. > > > > Signed-off-by: Sirio Balmelli <sirio@b-ad.ch> > > Thanks for the fix! One minor comment below. > > > --- > > tools/lib/bpf/libbpf.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 3dbe217bf23e..e2cc8f10c188 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -48,6 +48,8 @@ > > #include "bpf.h" > > #include "btf.h" > > > > +#include <inttypes.h> /* PRIi64 */ > > + > > #ifndef EM_BPF > > #define EM_BPF 247 > > #endif > > @@ -1042,7 +1044,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > > } > > > > if (def->key_size != key_size) { > > - pr_warning("map:%s key_type:%s has BTF type_size:%ld != key_size:%u\n", > > + pr_warning("map:%s key_type:%s has BTF type_size:%"PRIi64" != key_size:%u\n", > > map->name, name, key_size, def->key_size); > > return -EINVAL; > > } > > @@ -1069,7 +1071,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > > } > > > > if (def->value_size != value_size) { > > - pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", > > + pr_warning("map:%s value_type:%s has BTF type_size:%"PRIi64" != value_size:%u\n", > > map->name, name, value_size, def->value_size); > > I don't think we need the PRIi64 in here. Could you just change it to 'type_size:%u' > and then cast the 'key_size' resp. 'value_size' to unsigned int? We know at this > point that the type size value is positive anyway, so we only want to show the size > mismatch so simple cast should suffice. Thanks! Good point. 'unsigned int' is enough because the t->size (returned by btf_type_size()) is a u32. > > > return -EINVAL; > > } > > >
On Wed, May 23, 2018 at 08:49:47AM -0700, Martin KaFai Lau wrote: > On Wed, May 23, 2018 at 12:41:14PM +0200, Daniel Borkmann wrote: > > [ +Martin ] > > > > On 05/21/2018 08:59 AM, Sirio Balmelli wrote: > > > On arm32, 'cd tools/testing/selftests/bpf && make' fails with: > > > > > > libbpf.c:80:10: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘int64_t {aka long long int}’ [-Werror=format=] > > > (func)("libbpf: " fmt, ##__VA_ARGS__); \ > > > ^ > > > libbpf.c:83:30: note: in expansion of macro ‘__pr’ > > > #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__) > > > ^~~~ > > > libbpf.c:1072:3: note: in expansion of macro ‘pr_warning’ > > > pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", > > > > > > To fix, include 'inttypes.h' and change format directive to 'PRIi64'. > > > > > > Signed-off-by: Sirio Balmelli <sirio@b-ad.ch> > > > > Thanks for the fix! One minor comment below. > > > > > --- > > > tools/lib/bpf/libbpf.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > > index 3dbe217bf23e..e2cc8f10c188 100644 > > > --- a/tools/lib/bpf/libbpf.c > > > +++ b/tools/lib/bpf/libbpf.c > > > @@ -48,6 +48,8 @@ > > > #include "bpf.h" > > > #include "btf.h" > > > > > > +#include <inttypes.h> /* PRIi64 */ > > > + > > > #ifndef EM_BPF > > > #define EM_BPF 247 > > > #endif > > > @@ -1042,7 +1044,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > > > } > > > > > > if (def->key_size != key_size) { > > > - pr_warning("map:%s key_type:%s has BTF type_size:%ld != key_size:%u\n", > > > + pr_warning("map:%s key_type:%s has BTF type_size:%"PRIi64" != key_size:%u\n", > > > map->name, name, key_size, def->key_size); > > > return -EINVAL; > > > } > > > @@ -1069,7 +1071,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) > > > } > > > > > > if (def->value_size != value_size) { > > > - pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", > > > + pr_warning("map:%s value_type:%s has BTF type_size:%"PRIi64" != value_size:%u\n", > > > map->name, name, value_size, def->value_size); > > > > I don't think we need the PRIi64 in here. Could you just change it to 'type_size:%u' > > and then cast the 'key_size' resp. 'value_size' to unsigned int? We know at this > > point that the type size value is positive anyway, so we only want to show the size > > mismatch so simple cast should suffice. Thanks! > Good point. 'unsigned int' is enough because the t->size (returned > by btf_type_size()) is a u32. > > > > > > return -EINVAL; > > > } > > > > > Thank you very much; respinning patch.
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 3dbe217bf23e..e2cc8f10c188 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -48,6 +48,8 @@ #include "bpf.h" #include "btf.h" +#include <inttypes.h> /* PRIi64 */ + #ifndef EM_BPF #define EM_BPF 247 #endif @@ -1042,7 +1044,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) } if (def->key_size != key_size) { - pr_warning("map:%s key_type:%s has BTF type_size:%ld != key_size:%u\n", + pr_warning("map:%s key_type:%s has BTF type_size:%"PRIi64" != key_size:%u\n", map->name, name, key_size, def->key_size); return -EINVAL; } @@ -1069,7 +1071,7 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf) } if (def->value_size != value_size) { - pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", + pr_warning("map:%s value_type:%s has BTF type_size:%"PRIi64" != value_size:%u\n", map->name, name, value_size, def->value_size); return -EINVAL; }
On arm32, 'cd tools/testing/selftests/bpf && make' fails with: libbpf.c:80:10: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘int64_t {aka long long int}’ [-Werror=format=] (func)("libbpf: " fmt, ##__VA_ARGS__); \ ^ libbpf.c:83:30: note: in expansion of macro ‘__pr’ #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__) ^~~~ libbpf.c:1072:3: note: in expansion of macro ‘pr_warning’ pr_warning("map:%s value_type:%s has BTF type_size:%ld != value_size:%u\n", To fix, include 'inttypes.h' and change format directive to 'PRIi64'. Signed-off-by: Sirio Balmelli <sirio@b-ad.ch> --- tools/lib/bpf/libbpf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)