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

Cyrill Gorcunov gorcunov at gmail.com
Thu Sep 29 06:11:48 PDT 2016


On Thu, Sep 29, 2016 at 03:48:35PM +0300, Dmitry Safonov wrote:
> > --- /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?

We will need complete documentation for this, it's a parst of uapi,
so dont worry I'll add description. At the moment I simply need
to make a carcass of what will be in the library, moreover some
uapi might be changed or even completely rewritten.

> 
> > +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.

Maybe, at moment copied it from old compel code. Once everything
start working we could cleanup it a bit.


More information about the CRIU mailing list