On Sat, Mar 09, 2019 at 11:23:22AM +0100, Sebastian Benoit wrote:
> We free a few strings in main(), some others we dont. We should either
> free() all strings consistently or not free them at all, because it does not
> hurt to keep them until exit().
>
> The chances of the main() function being repurposed and that leading to
> memleaks is slim, readability is more important here, so this diff removes
> the free() calls.
>
> ok?
>
> (acme_no_free_in_main.diff)
remeber the malloc code does some consistentcy checks on free, so
this might cause misses of detection of memory management bugs,
-Otto
>
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/acme-client/main.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 main.c
> --- main.c 9 Mar 2019 10:11:53 -0000 1.44
> +++ main.c 9 Mar 2019 10:16:20 -0000
> @@ -238,7 +238,6 @@ main(int argc, char *argv[])
>
> if (pids[COMP_NET] == 0) {
> proccomp = COMP_NET;
> - free(certdir);
> close(key_fds[0]);
> close(acct_fds[0]);
> close(chng_fds[0]);
> @@ -252,7 +251,6 @@ main(int argc, char *argv[])
> dns_fds[1], rvk_fds[1],
> (popts & ACME_OPT_NEWACCT), revocate, authority,
> (const char *const *)alts, altsz);
> - free(alts);
> exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
> }
>
> @@ -270,7 +268,6 @@ main(int argc, char *argv[])
>
> if (pids[COMP_KEY] == 0) {
> proccomp = COMP_KEY;
> - free(certdir);
> close(cert_fds[0]);
> close(dns_fds[0]);
> close(rvk_fds[0]);
> @@ -280,7 +277,6 @@ main(int argc, char *argv[])
> close(file_fds[1]);
> c = keyproc(key_fds[0], domain->key,
> (const char **)alts, altsz, (popts & ACME_OPT_NEWDKEY));
> - free(alts);
> exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
> }
>
> @@ -293,8 +289,6 @@ main(int argc, char *argv[])
>
> if (pids[COMP_ACCOUNT] == 0) {
> proccomp = COMP_ACCOUNT;
> - free(certdir);
> - free(alts);
> close(cert_fds[0]);
> close(dns_fds[0]);
> close(rvk_fds[0]);
> @@ -314,8 +308,6 @@ main(int argc, char *argv[])
>
> if (pids[COMP_CHALLENGE] == 0) {
> proccomp = COMP_CHALLENGE;
> - free(certdir);
> - free(alts);
> close(cert_fds[0]);
> close(dns_fds[0]);
> close(rvk_fds[0]);
> @@ -334,8 +326,6 @@ main(int argc, char *argv[])
>
> if (pids[COMP_CERT] == 0) {
> proccomp = COMP_CERT;
> - free(certdir);
> - free(alts);
> close(dns_fds[0]);
> close(rvk_fds[0]);
> close(file_fds[1]);
> @@ -353,12 +343,10 @@ main(int argc, char *argv[])
>
> if (pids[COMP_FILE] == 0) {
> proccomp = COMP_FILE;
> - free(alts);
> close(dns_fds[0]);
> close(rvk_fds[0]);
> c = fileproc(file_fds[1], certdir, certfile, chainfile,
> fullchainfile);
> - free(certdir);
> /*
> * This is different from the other processes in that it
> * can return 2 if the certificates were updated.
> @@ -375,8 +363,6 @@ main(int argc, char *argv[])
>
> if (pids[COMP_DNS] == 0) {
> proccomp = COMP_DNS;
> - free(certdir);
> - free(alts);
> close(rvk_fds[0]);
> c = dnsproc(dns_fds[0]);
> exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
> @@ -395,8 +381,6 @@ main(int argc, char *argv[])
> certfile != NULL ? certfile : fullchainfile,
> force, revocate,
> (const char *const *)alts, altsz);
> - free(certdir);
> - free(alts);
> exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
> }
>
> @@ -421,11 +405,6 @@ main(int argc, char *argv[])
> checkexit(pids[COMP_DNS], COMP_DNS) +
> checkexit(pids[COMP_REVOKE], COMP_REVOKE);
>
> - free(certdir);
> - free(alts);
> - free(certfile);
> - free(chainfile);
> - free(fullchainfile);
> return rc != COMP__MAX ? EXIT_FAILURE : (c == 2 ? EXIT_SUCCESS : 2);
> usage:
> fprintf(stderr,
>
