[bpf-next,v2,1/8] libbpf: Improve log message formatting

Commit Message

Tony Ambardar Aug. 22, 2024, 9:24 a.m. UTC

From: Tony Ambardar <[e-mail]>

Fix missing newlines and extraneous terminal spaces in messages.

Signed-off-by: Tony Ambardar <[e-mail]>
---
 tools/lib/bpf/btf.c          | 7 ++++---
 tools/lib/bpf/btf_dump.c     | 2 +-
 tools/lib/bpf/btf_relocate.c | 2 +-
 tools/lib/bpf/libbpf.c       | 4 ++--
 tools/lib/bpf/relo_core.c    | 2 +-
 5 files changed, 9 insertions(+), 8 deletions(-)

Comments

Andrii Nakryiko Aug. 22, 2024, 11:36 p.m. UTC | #1

On Thu, Aug 22, 2024 at 2:24 AM Tony Ambardar <[e-mail]> wrote:
>
> From: Tony Ambardar <[e-mail]>
>
> Fix missing newlines and extraneous terminal spaces in messages.
>
> Signed-off-by: Tony Ambardar <[e-mail]>
> ---
>  tools/lib/bpf/btf.c          | 7 ++++---
>  tools/lib/bpf/btf_dump.c     | 2 +-
>  tools/lib/bpf/btf_relocate.c | 2 +-
>  tools/lib/bpf/libbpf.c       | 4 ++--
>  tools/lib/bpf/relo_core.c    | 2 +-
>  5 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 32c00db3b91b..cf4f7bd7ff5c 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -2940,7 +2940,8 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
>
>         /* If no records, return failure now so .BTF.ext won't be used. */
>         if (!info_left) {
> -               pr_debug("%s section in .BTF.ext has no records", ext_sec->desc);
> +               pr_debug("%s section in .BTF.ext has no records\n",
> +                        ext_sec->desc);

if it fits in 100 characters, it stays in a single line, please don't touch that

>                 return -EINVAL;
>         }
>
> @@ -3028,7 +3029,7 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
>
>         if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
>             data_size < hdr->hdr_len) {
> -               pr_debug("BTF.ext header not found");
> +               pr_debug("BTF.ext header not found\n");
>                 return -EINVAL;
>         }
>
> @@ -3290,7 +3291,7 @@ int btf__dedup(struct btf *btf, const struct btf_dedup_opts *opts)
>
>         d = btf_dedup_new(btf, opts);
>         if (IS_ERR(d)) {
> -               pr_debug("btf_dedup_new failed: %ld", PTR_ERR(d));
> +               pr_debug("btf_dedup_new failed: %ld\n", PTR_ERR(d));
>                 return libbpf_err(-EINVAL);
>         }
>
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index 894860111ddb..25e7c44d9f95 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -1304,7 +1304,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
>                          * chain, restore stack, emit warning, and try to
>                          * proceed nevertheless
>                          */
> -                       pr_warn("not enough memory for decl stack:%d", err);
> +                       pr_warn("not enough memory for decl stack:%d\n", err);

nit: add space after ':' while you are at it?

>                         d->decl_stack_cnt = stack_start;
>                         return;
>                 }

[...]

Tony Ambardar Aug. 26, 2024, 10:51 a.m. UTC | #2

On Thu, Aug 22, 2024 at 04:36:13PM -0700, Andrii Nakryiko wrote:
> On Thu, Aug 22, 2024 at 2:24 AM Tony Ambardar <[e-mail]> wrote:
> >
> > From: Tony Ambardar <[e-mail]>
> >
> > Fix missing newlines and extraneous terminal spaces in messages.
> >
> > Signed-off-by: Tony Ambardar <[e-mail]>
> > ---
> >  tools/lib/bpf/btf.c          | 7 ++++---
> >  tools/lib/bpf/btf_dump.c     | 2 +-
> >  tools/lib/bpf/btf_relocate.c | 2 +-
> >  tools/lib/bpf/libbpf.c       | 4 ++--
> >  tools/lib/bpf/relo_core.c    | 2 +-
> >  5 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 32c00db3b91b..cf4f7bd7ff5c 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -2940,7 +2940,8 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
> >
> >         /* If no records, return failure now so .BTF.ext won't be used. */
> >         if (!info_left) {
> > -               pr_debug("%s section in .BTF.ext has no records", ext_sec->desc);
> > +               pr_debug("%s section in .BTF.ext has no records\n",
> > +                        ext_sec->desc);
> 
> if it fits in 100 characters, it stays in a single line, please don't touch that

OK, will do. Most of these are due to CI and local checkpatch, plus kernel
docs on style/submission guidelines. It can be hard to decide which
warnings/errors can be ignored and which apply, so it would be helpful at
least if the CI flavour of checkpatch aligned with guidance above. Not sure
what that involves, but sure that others besides me would benefit.

> 
> >                 return -EINVAL;
> >         }
> >
> > @@ -3028,7 +3029,7 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
> >
> >         if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
> >             data_size < hdr->hdr_len) {
> > -               pr_debug("BTF.ext header not found");
> > +               pr_debug("BTF.ext header not found\n");
> >                 return -EINVAL;
> >         }
> >
> > @@ -3290,7 +3291,7 @@ int btf__dedup(struct btf *btf, const struct btf_dedup_opts *opts)
> >
> >         d = btf_dedup_new(btf, opts);
> >         if (IS_ERR(d)) {
> > -               pr_debug("btf_dedup_new failed: %ld", PTR_ERR(d));
> > +               pr_debug("btf_dedup_new failed: %ld\n", PTR_ERR(d));
> >                 return libbpf_err(-EINVAL);
> >         }
> >
> > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > index 894860111ddb..25e7c44d9f95 100644
> > --- a/tools/lib/bpf/btf_dump.c
> > +++ b/tools/lib/bpf/btf_dump.c
> > @@ -1304,7 +1304,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
> >                          * chain, restore stack, emit warning, and try to
> >                          * proceed nevertheless
> >                          */
> > -                       pr_warn("not enough memory for decl stack:%d", err);
> > +                       pr_warn("not enough memory for decl stack:%d\n", err);
> 
> nit: add space after ':' while you are at it?

Right, done. Thanks!

> 
> >                         d->decl_stack_cnt = stack_start;
> >                         return;
> >                 }
> 
> [...]

Patch

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 32c00db3b91b..cf4f7bd7ff5c 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -2940,7 +2940,8 @@  static int btf_ext_setup_info(struct btf_ext *btf_ext,
 
 	/* If no records, return failure now so .BTF.ext won't be used. */
 	if (!info_left) {
-		pr_debug("%s section in .BTF.ext has no records", ext_sec->desc);
+		pr_debug("%s section in .BTF.ext has no records\n",
+			 ext_sec->desc);
 		return -EINVAL;
 	}
 
@@ -3028,7 +3029,7 @@  static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
 
 	if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
 	    data_size < hdr->hdr_len) {
-		pr_debug("BTF.ext header not found");
+		pr_debug("BTF.ext header not found\n");
 		return -EINVAL;
 	}
 
@@ -3290,7 +3291,7 @@  int btf__dedup(struct btf *btf, const struct btf_dedup_opts *opts)
 
 	d = btf_dedup_new(btf, opts);
 	if (IS_ERR(d)) {
-		pr_debug("btf_dedup_new failed: %ld", PTR_ERR(d));
+		pr_debug("btf_dedup_new failed: %ld\n", PTR_ERR(d));
 		return libbpf_err(-EINVAL);
 	}
 
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 894860111ddb..25e7c44d9f95 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1304,7 +1304,7 @@  static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 			 * chain, restore stack, emit warning, and try to
 			 * proceed nevertheless
 			 */
-			pr_warn("not enough memory for decl stack:%d", err);
+			pr_warn("not enough memory for decl stack:%d\n", err);
 			d->decl_stack_cnt = stack_start;
 			return;
 		}
diff --git a/tools/lib/bpf/btf_relocate.c b/tools/lib/bpf/btf_relocate.c
index 4f7399d85eab..b72f83e15156 100644
--- a/tools/lib/bpf/btf_relocate.c
+++ b/tools/lib/bpf/btf_relocate.c
@@ -428,7 +428,7 @@  static int btf_relocate_rewrite_strs(struct btf_relocate *r, __u32 i)
 		} else {
 			off = r->str_map[*str_off];
 			if (!off) {
-				pr_warn("string '%s' [offset %u] is not mapped to base BTF",
+				pr_warn("string '%s' [offset %u] is not mapped to base BTF\n",
 					btf__str_by_offset(r->btf, off), *str_off);
 				return -ENOENT;
 			}
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e55353887439..8a0a0c1e37e1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -12753,7 +12753,7 @@  struct bpf_link *bpf_program__attach_freplace(const struct bpf_program *prog,
 	}
 
 	if (prog->type != BPF_PROG_TYPE_EXT) {
-		pr_warn("prog '%s': only BPF_PROG_TYPE_EXT can attach as freplace",
+		pr_warn("prog '%s': only BPF_PROG_TYPE_EXT can attach as freplace\n",
 			prog->name);
 		return libbpf_err_ptr(-EINVAL);
 	}
@@ -13843,7 +13843,7 @@  int bpf_object__open_subskeleton(struct bpf_object_subskeleton *s)
 		map_type = btf__type_by_id(btf, map_type_id);
 
 		if (!btf_is_datasec(map_type)) {
-			pr_warn("type for map '%1$s' is not a datasec: %2$s",
+			pr_warn("type for map '%1$s' is not a datasec: %2$s\n",
 				bpf_map__name(map),
 				__btf_kind_str(btf_kind(map_type)));
 			return libbpf_err(-EINVAL);
diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index 63a4d5ad12d1..7632e9d41827 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -1339,7 +1339,7 @@  int bpf_core_calc_relo_insn(const char *prog_name,
 					  cands->cands[i].id, cand_spec);
 		if (err < 0) {
 			bpf_core_format_spec(spec_buf, sizeof(spec_buf), cand_spec);
-			pr_warn("prog '%s': relo #%d: error matching candidate #%d %s: %d\n ",
+			pr_warn("prog '%s': relo #%d: error matching candidate #%d %s: %d\n",
 				prog_name, relo_idx, i, spec_buf, err);
 			return err;
 		}