[pacman-dev] [PATCH v3 4/5] pacman-conf.c: Make cleanup run automagically at exit

[Andrew Gregory]

On 02/10/18 at 10:15pm, iff at escondida.tk wrote:
> From: Ivy Foster <iff at escondida.tk>
> 
> Since using atexit(3) draws in stdlib anyway and this changes every
> exit call at large and return call in main(), go ahead and use
> EXIT_SUCCESS and EXIt_FAILURE, too.
> 
> Signed-off-by: Ivy Foster <iff at escondida.tk>
> ---
>  src/pacman/pacman-conf.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c
> index 8985b326..51df8803 100644
> --- a/src/pacman/pacman-conf.c
> +++ b/src/pacman/pacman-conf.c
> @@ -18,7 +18,9 @@
>   */
>  
>  #include <getopt.h>
> +#include <stdlib.h>
>  #include <string.h>
> +
>  #include "conf.h"
>  
>  #define MYNAME "pacman-conf"
> @@ -88,19 +90,14 @@ static void parse_opts(int argc, char **argv)
>  				break;
>  			case 'h':
>  				usage(stdout);
> -				cleanup();
> -				exit(0);
> +				exit(EXIT_SUCCESS);
>  			case 'V':
>  				printf("%s v%s\n", MYNAME, MYVER);
> -				cleanup();
> -				exit(0);
> -				break;
> +				exit(EXIT_SUCCESS);
>  			case '?':
>  			default:
>  				usage(stderr);
> -				cleanup();
> -				exit(1);
> -				break;
> +				exit(EXIT_FAILURE);
>  		}
>  	}
>  
> @@ -401,11 +398,15 @@ int main(int argc, char **argv)
>  {
>  	int ret = 0;
>  
> +	if (atexit(cleanup) != 0) {
> +		fputs("error: cannot set exit function\n", stderr);
> +		return EXIT_FAILURE;
> +	}

You're registering cleanup prior to initializing config.  At this
point in the patchset pacman-conf can't exit between the two, but
a later patch delays the initialization, which can lead to a segfault.

>  	config = config_new();
>  	parse_opts(argc, argv);
>  	if(!config) {
> -		ret = 1;
> -		goto cleanup;
> +		return EXIT_FAILURE;
>  	}
>  
>  	for(; optind < argc; optind++) {
> @@ -419,8 +420,7 @@ int main(int argc, char **argv)
>  	if(repo_list) {
>  		if(directives) {
>  			fputs("error: directives may not be specified with --repo-list\n", stderr);
> -			ret = 1;
> -			goto cleanup;
> +			return EXIT_FAILURE;
>  		}
>  		list_repos();
>  	} else if(repo_name) {
> @@ -429,10 +429,7 @@ int main(int argc, char **argv)
>  		ret = list_directives();
>  	}
>  
> -cleanup:
> -	cleanup();
> -
> -	return ret;
> +	return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>  }
>  
>  /* vim: set ts=2 sw=2 noet: */
> -- 
> 2.16.1

-- 
apg