Re: [PATCH v3] pkl: Remove global state from IOS

[Jose E. Marchesi]

Hi Mohammad.

Thanks for the patch.  Please see comments below.

> diff --git a/libpoke/ios-dev-sub.c b/libpoke/ios-dev-sub.c
> index f6646dab..7b798c0d 100644
> --- a/libpoke/ios-dev-sub.c
> +++ b/libpoke/ios-dev-sub.c
> @@ -32,6 +32,7 @@
>  
>  struct ios_dev_sub
>  {
> +  ios_context ictx;

Please use a more significant name, like ios_ctxt.  Ditto in other
places.

> diff --git a/libpoke/ios.c b/libpoke/ios.c
> index 6bc5bd12..3bce3c93 100644
> --- a/libpoke/ios.c
> +++ b/libpoke/ios.c
> @@ -78,15 +78,6 @@ struct ios
>    struct ios *next;
>  };
>  
> -/* Next available IOS id.  */
> -
> -static int ios_next_id = 0;
> -
> -/* List of IO spaces, and pointer to the current one.  */
> -
> -static struct ios *io_list;
> -static struct ios *cur_io;
> -
>  /* The available backends are implemented in their own files, and
>     provide the following interfaces.  */
>  
> @@ -102,83 +93,116 @@ extern struct ios_dev_if ios_dev_proc; /* 
> ios-dev-proc.c */
>  #endif
>  extern struct ios_dev_if ios_dev_sub; /* ios-dev-sub.c */
>  
> -static struct ios_dev_if *ios_dev_ifs[] =
> -  {
> -   NULL, /* Optional foreign IOD.  */
> -   &ios_dev_zero,
> -   &ios_dev_mem,
> -   &ios_dev_stream,
> +enum
> +{
> +  IOS_DEV_FOREIGN, /* Foreign must be first */
> +  IOS_DEV_ZERO,
> +  IOS_DEV_MEM,
> +  IOS_DEV_STREAM,
> +  IOS_DEV_NBD,
> +  IOS_DEV_PROC,
> +  IOS_DEV_SUB,
> +  IOS_DEV_FILE, /* File must be last */
> +
> +  IOS_DEV_COUNT
> +};
> +
> +struct ios_context
> +{
> +  int next_id;         /* Next available IOS id.  */
> +  struct ios *io_list; /* List of all IO spaces.  */
> +  struct ios *cur_io;  /* Pointer to the current IOS.  */
> +  struct ios_dev_if dev_ifs[IOS_DEV_COUNT]; /* IO devices.  */
> +  int dev_ifs_valid_p[IOS_DEV_COUNT];       /* Validity of IO devices.  */
> +};
> +
> +ios_context
> +ios_init (void)
> +{
> +  ios_context ictx = calloc (1, sizeof (struct ios_context));
> +
> +  if (!ictx)
> +    return NULL;
> +
> +#define DEV_IF(idx, val)                                                     
>  \
> +  do                                                                         
>  \
> +    {                                                                        
>  \
> +      memcpy (&ictx->dev_ifs[IOS_DEV_##idx], &ios_dev_##val,                 
>  \
> +              sizeof (struct ios_dev_if));                                   
>  \
> +      ictx->dev_ifs_valid_p[IOS_DEV_##idx] = 1;                              
>  \
> +    }                                                                        
>  \
> +  while (0)

Hmm, I don't like the fact we are copying the ios_dev_if structures
around.  Looks too complicated.  Is this because they hold `data'
pointers per ios context?

> +
> +  DEV_IF(ZERO, zero);
> +  DEV_IF(MEM, mem);
> +  DEV_IF(STREAM, stream);
>  #ifdef HAVE_LIBNBD
> -   &ios_dev_nbd,
> +  DEV_IF(NBD, nbd);
>  #endif
>  #ifdef HAVE_PROC
> -   &ios_dev_proc,
> +  DEV_IF(PROC, proc);
>  #endif
> -   &ios_dev_sub,
> -   /* File must be last */
> -   &ios_dev_file,
> -   NULL,
> -  };
> +  DEV_IF(SUB, sub);
> +  ictx->dev_ifs[IOS_DEV_SUB].data = ictx;
> +  DEV_IF(FILE, file);