diff mbox series

[2/2] dwarves: cus__load_files: set errno if load fails

Message ID 20220316132354.3226908-1-kkourt@kkourt.io (mailing list archive)
State Not Applicable
Delegated to: BPF
Headers show
Series [1/2] pahole: avoid segfault when parsing bogus file | expand

Checks

Context Check Description
bpf/vmtest-bpf-PR fail merge-conflict
netdev/tree_selection success Not a local patch

Commit Message

Kornilios Kourtis March 16, 2022, 1:23 p.m. UTC
From: Kornilios Kourtis <kornilios@isovalent.com>

This patch improves the error seen by the user by setting errno in
cus__load_files(). Otherwise, we get a "No such file or directory" error
which might be confusing.

Before the patch, using a bogus file:
$ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
pahole: ./vmlinux-5.3.18-24.102-default.debug: No such file or directory
$ ls ./vmlinux-5.3.18-24.102-default.debug
/home/kkourt/src/hubble-fgs/vmlinux-5.3.18-24.102-default.debug

After the patch:
$ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
pahole: ./vmlinux-5.3.18-24.102-default.debug: Unknown error -22

Which is not very helpful, but less confusing.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
---
 dwarves.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Arnaldo Carvalho de Melo March 16, 2022, 8:51 p.m. UTC | #1
Em Wed, Mar 16, 2022 at 02:23:54PM +0100, kkourt@kkourt.io escreveu:
> From: Kornilios Kourtis <kornilios@isovalent.com>
> 
> This patch improves the error seen by the user by setting errno in
> cus__load_files(). Otherwise, we get a "No such file or directory" error
> which might be confusing.
> 
> Before the patch, using a bogus file:
> $ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
> pahole: ./vmlinux-5.3.18-24.102-default.debug: No such file or directory
> $ ls ./vmlinux-5.3.18-24.102-default.debug
> /home/kkourt/src/hubble-fgs/vmlinux-5.3.18-24.102-default.debug
> 
> After the patch:
> $ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
> pahole: ./vmlinux-5.3.18-24.102-default.debug: Unknown error -22
> 
> Which is not very helpful, but less confusing.

Humm, because you should've set errno to -err back in cus__load_files(),
with this on top of your two patches we should get the:

#define EINVAL          22      /* Invalid argument */


"Invalid argument" or so from getting.

diff --git a/dwarves.c b/dwarves.c
index 5d0b420f0110452e..89609e96c46747ce 100644
--- a/dwarves.c
+++ b/dwarves.c
@@ -2401,7 +2401,7 @@ int cus__load_files(struct cus *cus, struct conf_load *conf,
 	while (filenames[i] != NULL) {
 		int err = cus__load_file(cus, conf, filenames[i]);
 		if (err) {
-			errno = err;
+			errno = -err;
 			return -++i;
 		}
 		++i;



Agreed? I'll fix it up here and apply if so.

- Arnaldo
 
> Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
> ---
>  dwarves.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/dwarves.c b/dwarves.c
> index 89b58ef..5d0b420 100644
> --- a/dwarves.c
> +++ b/dwarves.c
> @@ -2399,8 +2399,11 @@ int cus__load_files(struct cus *cus, struct conf_load *conf,
>  	int i = 0;
>  
>  	while (filenames[i] != NULL) {
> -		if (cus__load_file(cus, conf, filenames[i]))
> +		int err = cus__load_file(cus, conf, filenames[i]);
> +		if (err) {
> +			errno = err;
>  			return -++i;
> +		}
>  		++i;
>  	}
>  
> -- 
> 2.25.1
Kornilios Kourtis March 16, 2022, 8:55 p.m. UTC | #2
On Wed, Mar 16, 2022 at 05:51:03PM -0300, Arnaldo Carvalho de Melo wrote:
> Agreed? I'll fix it up here and apply if so.

Agreed, thanks!

cheers,
Kornilios.
John Fastabend March 17, 2022, 5 a.m. UTC | #3
kkourt@ wrote:
> From: Kornilios Kourtis <kornilios@isovalent.com>
> 
> This patch improves the error seen by the user by setting errno in
> cus__load_files(). Otherwise, we get a "No such file or directory" error
> which might be confusing.
> 
> Before the patch, using a bogus file:
> $ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
> pahole: ./vmlinux-5.3.18-24.102-default.debug: No such file or directory
> $ ls ./vmlinux-5.3.18-24.102-default.debug
> /home/kkourt/src/hubble-fgs/vmlinux-5.3.18-24.102-default.debug
> 
> After the patch:
> $ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
> pahole: ./vmlinux-5.3.18-24.102-default.debug: Unknown error -22
> 
> Which is not very helpful, but less confusing.
> 
> Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
> ---

With the err to -err fix Arnaldo proposed.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Arnaldo Carvalho de Melo March 17, 2022, 3:19 p.m. UTC | #4
Em Wed, Mar 16, 2022 at 10:00:57PM -0700, John Fastabend escreveu:
> kkourt@ wrote:
> > From: Kornilios Kourtis <kornilios@isovalent.com>
> > 
> > This patch improves the error seen by the user by setting errno in
> > cus__load_files(). Otherwise, we get a "No such file or directory" error
> > which might be confusing.
> > 
> > Before the patch, using a bogus file:
> > $ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
> > pahole: ./vmlinux-5.3.18-24.102-default.debug: No such file or directory
> > $ ls ./vmlinux-5.3.18-24.102-default.debug
> > /home/kkourt/src/hubble-fgs/vmlinux-5.3.18-24.102-default.debug
> > 
> > After the patch:
> > $ ./pahole -J ./vmlinux-5.3.18-24.102-default.debug
> > pahole: ./vmlinux-5.3.18-24.102-default.debug: Unknown error -22
> > 
> > Which is not very helpful, but less confusing.
> > 
> > Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
> > ---
> 
> With the err to -err fix Arnaldo proposed.
> 
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Thanks, did the ammendment and collected your Acked-by,

- Arnaldo
diff mbox series

Patch

diff --git a/dwarves.c b/dwarves.c
index 89b58ef..5d0b420 100644
--- a/dwarves.c
+++ b/dwarves.c
@@ -2399,8 +2399,11 @@  int cus__load_files(struct cus *cus, struct conf_load *conf,
 	int i = 0;
 
 	while (filenames[i] != NULL) {
-		if (cus__load_file(cus, conf, filenames[i]))
+		int err = cus__load_file(cus, conf, filenames[i]);
+		if (err) {
+			errno = err;
 			return -++i;
+		}
 		++i;
 	}