Re: [PATCH 1/3] libselinux: simplify string copying

[James Carter <jwcart2@gmail.com>]

On Wed, Nov 9, 2022 at 3:11 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Use strdup(3)/strndup(3) instead of allocating memory and then manually
> copying the content.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

For these three patches:
Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  libselinux/src/context.c                     | 11 +++++------
>  libselinux/src/get_default_type.c            |  3 +--
>  libselinux/src/matchpathcon.c                |  9 +++------
>  libselinux/utils/selabel_lookup_best_match.c | 10 ++++------
>  4 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/libselinux/src/context.c b/libselinux/src/context.c
> index 9dddbc5a..8830bf42 100644
> --- a/libselinux/src/context.c
> +++ b/libselinux/src/context.c
> @@ -149,19 +149,18 @@ static int set_comp(context_private_t * n, int idx, const char *str)
>         char *t = NULL;
>         const char *p;
>         if (str) {
> -               t = (char *)malloc(strlen(str) + 1);
> -               if (!t) {
> -                       return -1;
> -               }
>                 for (p = str; *p; p++) {
>                         if (*p == '\t' || *p == '\n' || *p == '\r' ||
>                             ((*p == ':' || *p == ' ') && idx != COMP_RANGE)) {
> -                               free(t);
>                                 errno = EINVAL;
>                                 return -1;
>                         }
>                 }
> -               strcpy(t, str);
> +
> +               t = strdup(str);
> +               if (!t) {
> +                       return -1;
> +               }
>         }
>         conditional_free(&n->component[idx]);
>         n->component[idx] = t;
> diff --git a/libselinux/src/get_default_type.c b/libselinux/src/get_default_type.c
> index dd7b5d79..766ea4b7 100644
> --- a/libselinux/src/get_default_type.c
> +++ b/libselinux/src/get_default_type.c
> @@ -62,10 +62,9 @@ static int find_default_type(FILE * fp, const char *role, char **type)
>                 return -1;
>         }
>
> -       t = malloc(strlen(buf) - len);
> +       t = strndup(ptr, strlen(buf) - len - 1);
>         if (!t)
>                 return -1;
> -       strcpy(t, ptr);
>         *type = t;
>         return 0;
>  }
> diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
> index ea78a23e..bf2da083 100644
> --- a/libselinux/src/matchpathcon.c
> +++ b/libselinux/src/matchpathcon.c
> @@ -215,10 +215,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
>                         if (ret < 0 || sb.st_ino != ino) {
>                                 fl->specind = specind;
>                                 free(fl->file);
> -                               fl->file = malloc(strlen(file) + 1);
> +                               fl->file = strdup(file);
>                                 if (!fl->file)
>                                         goto oom;
> -                               strcpy(fl->file, file);
>                                 return fl->specind;
>
>                         }
> @@ -232,10 +231,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
>                              __FUNCTION__, file, fl->file,
>                              con_array[fl->specind]);
>                         free(fl->file);
> -                       fl->file = malloc(strlen(file) + 1);
> +                       fl->file = strdup(file);
>                         if (!fl->file)
>                                 goto oom;
> -                       strcpy(fl->file, file);
>                         return fl->specind;
>                 }
>
> @@ -248,10 +246,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
>                 goto oom;
>         fl->ino = ino;
>         fl->specind = specind;
> -       fl->file = malloc(strlen(file) + 1);
> +       fl->file = strdup(file);
>         if (!fl->file)
>                 goto oom_freefl;
> -       strcpy(fl->file, file);
>         fl->next = prevfl->next;
>         prevfl->next = fl;
>         return fl->specind;
> diff --git a/libselinux/utils/selabel_lookup_best_match.c b/libselinux/utils/selabel_lookup_best_match.c
> index a4af0679..e816c04b 100644
> --- a/libselinux/utils/selabel_lookup_best_match.c
> +++ b/libselinux/utils/selabel_lookup_best_match.c
> @@ -30,7 +30,7 @@ static __attribute__ ((__noreturn__)) void usage(const char *progname)
>         exit(1);
>  }
>
> -static mode_t string_to_mode(char *s)
> +static mode_t string_to_mode(const char *s)
>  {
>         switch (s[0]) {
>         case 'b':
> @@ -53,7 +53,7 @@ static mode_t string_to_mode(char *s)
>
>  int main(int argc, char **argv)
>  {
> -       int raw = 0, mode = 0, rc, opt, i, num_links, string_len;
> +       int raw = 0, mode = 0, rc, opt, i, num_links;
>         char *validate = NULL, *path = NULL, *context = NULL, *file = NULL;
>         char **links = NULL;
>
> @@ -101,13 +101,11 @@ int main(int argc, char **argv)
>                 }
>
>                 for (i = optind, num_links = 0; i < argc; i++, num_links++) {
> -                       string_len = strlen(argv[i]) + 1;
> -                       links[num_links] = malloc(string_len);
> +                       links[num_links] = strdup(argv[i]);
>                         if (!links[num_links]) {
> -                               fprintf(stderr, "ERROR: malloc failed.\n");
> +                               fprintf(stderr, "ERROR: strdup failed.\n");
>                                 exit(1);
>                         }
> -                       strcpy(links[num_links], argv[i]);
>                 }
>         }
>
> --
> 2.38.1
>