[CRIU] [PATCH 3/6] compel: Add arguments packing helper

[Dmitry Safonov]

Hi Cyrill,

2016-09-29 0:34 GMT+03:00 Cyrill Gorcunov <gorcunov at openvz.org>:
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  compel/Makefile              |  2 ++
>  compel/include/uapi/compel.h |  4 ++++
>  compel/src/lib/argv.c        | 39 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 45 insertions(+)
>  create mode 100644 compel/src/lib/argv.c
>
> diff --git a/compel/Makefile b/compel/Makefile
> index bfb79237dbb2..eb46393f3355 100644
> --- a/compel/Makefile
> +++ b/compel/Makefile
> @@ -15,6 +15,8 @@ host-lib-y            += arch/$(ARCH)/src/lib/handle-elf.o
>  lib-y                  += src/lib/handle-elf.o
>  host-lib-y             += src/lib/handle-elf.o
>
> +lib-y                  += src/lib/argv.o
> +
>  ifeq ($(ARCH),x86)
>  lib-y                  += src/lib/handle-elf-32.o
>  host-lib-y             += src/lib/handle-elf-32.o
> diff --git a/compel/include/uapi/compel.h b/compel/include/uapi/compel.h
> index 10507ce3fb22..9148f94bda45 100644
> --- a/compel/include/uapi/compel.h
> +++ b/compel/include/uapi/compel.h
> @@ -14,4 +14,8 @@ typedef struct {
>         long            value;
>  } compel_reloc_t;
>
> +extern int libcompel_pack_argv(void *blob, size_t blob_size,
> +                              int argc, char **argv,
> +                              void **arg_p, size_t *arg_size);
> +
>  #endif /* UAPI_COMPEL_H__ */
> diff --git a/compel/src/lib/argv.c b/compel/src/lib/argv.c
> new file mode 100644
> index 000000000000..86f354e83679
> --- /dev/null
> +++ b/compel/src/lib/argv.c
> @@ -0,0 +1,39 @@
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "uapi/compel.h"
> +

Maybe we could have some comment here (a patch on top of this):
while I belive, I understand that here we pack arguments by
specifing their nr, and arguments length before argv, it's still a bit
confusing to stare at this.
It also packs blob as argv[0], so could we reflect that somewhere?

> +int libcompel_pack_argv(void *blob, size_t blob_size,
> +                       int argc, char **argv,
> +                       void **arg_p, size_t *arg_size)
> +{
> +       unsigned long *argv_packed;
> +       size_t total_len;
> +       int i, *argc_packed;
> +       char *args_mem;
> +
> +       total_len = sizeof(int);
> +       total_len += sizeof(unsigned long) + blob_size + 1;
> +       for (i = 0; i < argc; i++)
> +               total_len += sizeof(unsigned long) + strlen(argv[i]) + 1;
> +
> +       argc_packed = malloc(total_len);
> +       if (!argc_packed)
> +               return -ENOMEM;
> +
> +       *argc_packed = argc + 1;
> +       argv_packed = (unsigned long *)(argc_packed + 1);

Maybe have argc_packed as (unsigned long*) then (int*)?
Just to pack with the same type, eliminating casts.
It maybe worth also to add
if (argc < 0)
  return -EINVAL;
then to store some crapy counter+1 value.

> +       args_mem = (char *)(argv_packed + argc + 1);
> +       argv_packed[0] = (void *)args_mem - (void *)argc_packed;
> +       memcpy(args_mem, blob, blob_size);
> +       args_mem += blob_size + 1;
> +       for (i = 0; i < argc; i++) {
> +               argv_packed[i + 1] = (void *)args_mem - (void *)argc_packed;
> +               args_mem = stpcpy(args_mem, argv[i]) + 1;
> +       }
> +
> +       *arg_p = argc_packed;
> +       *arg_size = total_len;
> +
> +       return 0;
> +}
> --
> 2.7.4
>
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu



-- 
             Dmitry