On Fri, Aug 6, 2021 at 12:23 PM Alice Mitchell <ajmitchell@xxxxxxxxxx> wrote:
>
> Also fix the modification of a returned config value which
> should be treated as const.
>
> Signed-off-by: Alice Mitchell <ajmitchell@xxxxxxxxxx>
> ---
> utils/gssd/gssd.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
> index 4113cba..0815665 100644
> --- a/utils/gssd/gssd.c
> +++ b/utils/gssd/gssd.c
> @@ -1016,7 +1016,7 @@ read_gss_conf(void)
> keytabfile = s;
> s = conf_get_str("gssd", "cred-cache-directory");
> if (s)
> - ccachedir = s;
> + ccachedir = strdup(s);
> s = conf_get_str("gssd", "preferred-realm");
> if (s)
> preferred_realm = s;
> @@ -1070,7 +1070,7 @@ main(int argc, char *argv[])
> keytabfile = optarg;
> break;
> case 'd':
> - ccachedir = optarg;
> + ccachedir = strdup(optarg);
Is it possible that there will be a value in both config file and
command line args. If we are strdup-ing in both we'll be over-writting
and leaking memory?
Why do we need to malloc it at all? Is it ever malloc-ed now (and
considered a leak)? I think in both cases it uses static memory and
doesnt require freeing.
> break;
> case 't':
> context_timeout = atoi(optarg);
> @@ -1133,7 +1133,6 @@ main(int argc, char *argv[])
> }
>
> if (ccachedir) {
> - char *ccachedir_copy;
> char *ptr;
>
> for (ptr = ccachedir, i = 2; *ptr; ptr++)
> @@ -1141,8 +1140,7 @@ main(int argc, char *argv[])
> i++;
>
> ccachesearch = malloc(i * sizeof(char *));
> - ccachedir_copy = strdup(ccachedir);
> - if (!ccachedir_copy || !ccachesearch) {
> + if (!ccachesearch) {
ccachedir_copy is the only leak here.
> printerr(0, "malloc failure\n");
> exit(EXIT_FAILURE);
> }
> @@ -1274,6 +1272,8 @@ main(int argc, char *argv[])
>
> free(preferred_realm);
> free(ccachesearch);
> + if (ccachedir)
> + free(ccachedir);
>
> return rc < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
> }
> --
> 2.27.0
>
>
