diff mbox series

sparc: Move va_end() before exit()

Message ID 20241206092346.6003-1-liujing@cmss.chinamobile.com
State New
Headers show
Series sparc: Move va_end() before exit() | expand

Commit Message

liujing Dec. 6, 2024, 9:23 a.m. UTC
There is a static checker warning, so move the va_end call before
exit(1). Since the exit(1) function terminates the program, any code
after exit(1) is unreachable thus notexecuted. Placing va_end() before
exit() ensures that the va_list is properly cleaned up.

Signed-off-by: liujing <liujing@cmss.chinamobile.com>
---
V1 -> V2: Modify the commit information and title description

Comments

Andreas Larsson Dec. 6, 2024, 10:45 a.m. UTC | #1
On 2024-12-06 10:23, liujing wrote:
> [PATCH] sparc: Move va_end() before exit()

That this is version 2 of the patch should be indicated in the subject
line as [PATCH v2]. You can use -v <n> or --reroll-count=<n> with git
format-patch to do it for you. Make sure to do that for your v3.

The description line still doesn't indicate where under sparc this
change is happening. At least start with "sparc: vdso: ...".

Thanks,
Andreas
David Laight Dec. 8, 2024, 11:03 a.m. UTC | #2
From: liujing
> Sent: 06 December 2024 09:24
> 
> There is a static checker warning, so move the va_end call before
> exit(1). Since the exit(1) function terminates the program, any code
> after exit(1) is unreachable thus notexecuted. Placing va_end() before
> exit() ensures that the va_list is properly cleaned up.

Aren't you just adding 'bloat' to the vdso?
(It might even be space limited?)

If you are calling exit() then there is no need to tidy up the va_list.
Much the same as there is no need to call free() or close() all fd.

I'd probably have written it as:
	// va_end(ap);
	exit(1);

	David
> 
> Signed-off-by: liujing <liujing@cmss.chinamobile.com>
> ---
> V1 -> V2: Modify the commit information and title description
> 
> diff --git a/arch/sparc/vdso/vdso2c.c b/arch/sparc/vdso/vdso2c.c
> index dc81240aab6f..372e3330850a 100644
> --- a/arch/sparc/vdso/vdso2c.c
> +++ b/arch/sparc/vdso/vdso2c.c
> @@ -90,8 +90,8 @@ static void fail(const char *format, ...)
>  	vfprintf(stderr, format, ap);
>  	if (outfilename)
>  		unlink(outfilename);
> -	exit(1);
>  	va_end(ap);
> +	exit(1);
>  }
> 
>  /*
> --
> 2.27.0
> 
> 
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Thomas Weißschuh Dec. 8, 2024, 12:17 p.m. UTC | #3
Hi David,

On 2024-12-08 11:03:23+0000, David Laight wrote:
> From: liujing
> > Sent: 06 December 2024 09:24
> > 
> > There is a static checker warning, so move the va_end call before
> > exit(1). Since the exit(1) function terminates the program, any code
> > after exit(1) is unreachable thus notexecuted. Placing va_end() before
> > exit() ensures that the va_list is properly cleaned up.
> 
> Aren't you just adding 'bloat' to the vdso?
> (It might even be space limited?)

This is about the build time tool "vdso2c", which converts the VDSO
binary into a C file to be compiled and linked into the kernel.
So it can't be a runtime issue.

That should probably been in the patch subject.

> If you are calling exit() then there is no need to tidy up the va_list.
> Much the same as there is no need to call free() or close() all fd.
> 
> I'd probably have written it as:
> 	// va_end(ap);
> 	exit(1);
> 
> 	David
> > 
> > Signed-off-by: liujing <liujing@cmss.chinamobile.com>
> > ---
> > V1 -> V2: Modify the commit information and title description
> > 
> > diff --git a/arch/sparc/vdso/vdso2c.c b/arch/sparc/vdso/vdso2c.c
> > index dc81240aab6f..372e3330850a 100644
> > --- a/arch/sparc/vdso/vdso2c.c
> > +++ b/arch/sparc/vdso/vdso2c.c
> > @@ -90,8 +90,8 @@ static void fail(const char *format, ...)
> >  	vfprintf(stderr, format, ap);
> >  	if (outfilename)
> >  		unlink(outfilename);
> > -	exit(1);
> >  	va_end(ap);
> > +	exit(1);
> >  }
> > 
> >  /*
> > --
> > 2.27.0
diff mbox series

Patch

diff --git a/arch/sparc/vdso/vdso2c.c b/arch/sparc/vdso/vdso2c.c
index dc81240aab6f..372e3330850a 100644
--- a/arch/sparc/vdso/vdso2c.c
+++ b/arch/sparc/vdso/vdso2c.c
@@ -90,8 +90,8 @@  static void fail(const char *format, ...)
 	vfprintf(stderr, format, ap);
 	if (outfilename)
 		unlink(outfilename);
-	exit(1);
 	va_end(ap);
+	exit(1);
 }
 
 /*