Re: [PATCH 2/2] ios: combine all global states into a single struct

[Mohammad-Reza Nabipoor]

ping

On Wed, Aug 10, 2022 at 02:39:27AM +0430, Mohammad-Reza Nabipoor wrote:
> 2022-08-10  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> 
>       * libpoke/ios-dev.h (struct ios_dev_if): Remove `data' field.
>       * libpoke/ios.h (ios_register_foreign_iod): Add new param: `data'.
>       * libpoke/ios.c (struct ios_context): New struct to put all
>       global states in one place.
>       (IOS_DEV_IF_DATA): New macro.
>       (ios_ctx): An instance of `struct ios_context' which contains
>       all global states.
>       (ios_shutdown): Adapt to use `ios_ctx' global variable.
>       (ios_open): Likewise.
>       (ios_close): Likewise.
>       (ios_cur): Likewise.
>       (ios_set_cur): Likewise.
>       (ios_search): Likewise.
>       (ios_search_by_id): Likewise.
>       (ios_set_bias): Likewise.
>       (ios_map): Likewise.
>       (ios_register_foreign_iod): Use new param.
>       * libpoke/libpoke.c (pk_register_iod): Update.
> ---
> 
> Hello Jose.
> 
> This is a step toward removing global state from libpoke.
> 
> 
> Regards,
> Mohammad-Reza
> 
>  
>  ChangeLog         | 21 +++++++++++++++
>  libpoke/ios-dev.h |  1 -
>  libpoke/ios.c     | 66 ++++++++++++++++++++++++++---------------------
>  libpoke/ios.h     |  2 +-
>  libpoke/libpoke.c |  3 +--
>  5 files changed, 60 insertions(+), 33 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 834441ee..9d700d13 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,24 @@
> +2022-08-10  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * libpoke/ios-dev.h (struct ios_dev_if): Remove `data' field.
> +     * libpoke/ios.h (ios_register_foreign_iod): Add new param: `data'.
> +     * libpoke/ios.c (struct ios_context): New struct to put all
> +     global states in one place.
> +     (IOS_DEV_IF_DATA): New macro.
> +     (ios_ctx): An instance of `struct ios_context' which contains
> +     all global states.
> +     (ios_shutdown): Adapt to use `ios_ctx' global variable.
> +     (ios_open): Likewise.
> +     (ios_close): Likewise.
> +     (ios_cur): Likewise.
> +     (ios_set_cur): Likewise.
> +     (ios_search): Likewise.
> +     (ios_search_by_id): Likewise.
> +     (ios_set_bias): Likewise.
> +     (ios_map): Likewise.
> +     (ios_register_foreign_iod): Use new param.
> +     * libpoke/libpoke.c (pk_register_iod): Update.
> +
>  2022-08-10  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>  
>       * bootstrap.conf (libpoke_modules): Add `array-list'.
> diff --git a/libpoke/ios-dev.h b/libpoke/ios-dev.h
> index 3be1c295..71628504 100644
> --- a/libpoke/ios-dev.h
> +++ b/libpoke/ios-dev.h
> @@ -63,7 +63,6 @@ struct ios_dev_if
>    uint64_t (*get_flags) (void *dev);
>    ios_dev_off (*size) (void *dev);
>    int (*flush) (void *dev, ios_dev_off offset);
> -  void *data;
>  };
>  
>  #define IOS_FILE_HANDLER_NORMALIZE(handler, new_handler)                \
> diff --git a/libpoke/ios.c b/libpoke/ios.c
> index 464a75b4..a79ff4d8 100644
> --- a/libpoke/ios.c
> +++ b/libpoke/ios.c
> @@ -80,14 +80,17 @@ 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.  */
> +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.  */
> +  void *foreign_dev_if_data; /* User-defined data for foreign device.  */
> +};
>  
> -static struct ios *io_list;
> -static struct ios *cur_io;
> +/* Only foreign device supports user-defined data.  */
> +#define IOS_DEV_IF_DATA(dev_if)                                              
>  \
> +  ((dev_if) == ios_dev_ifs[0] ? ios_ctx.foreign_dev_if_data : NULL)
>  
>  /* The available backends are implemented in their own files, and
>     provide the following interfaces.  */
> @@ -122,6 +125,8 @@ static struct ios_dev_if *ios_dev_ifs[] =
>     NULL,
>    };
>  
> +static struct ios_context ios_ctx;
> +
>  void
>  ios_init (void)
>  {
> @@ -132,8 +137,8 @@ void
>  ios_shutdown (void)
>  {
>    /* Close and free all open IO spaces.  */
> -  while (io_list)
> -    ios_close (io_list);
> +  while (ios_ctx.io_list)
> +    ios_close (ios_ctx.io_list);
>  }
>  
>  int
> @@ -181,7 +186,7 @@ ios_open (const char *handler, uint64_t flags, int 
> set_cur)
>    io->dev_if = *dev_if;
>  
>    /* Do not re-open an already-open IO space.  */
> -  for (ios i = io_list; i; i = i->next)
> +  for (ios i = ios_ctx.io_list; i; i = i->next)
>      if (STREQ (i->handler, io->handler))
>        {
>          error = IOS_EOPEN;
> @@ -189,20 +194,21 @@ ios_open (const char *handler, uint64_t flags, int 
> set_cur)
>        }
>  
>    /* Open the device using the interface found above.  */
> -  io->dev = io->dev_if->open (handler, flags, &iod_error, io->dev_if->data);
> +  io->dev = io->dev_if->open (handler, flags, &iod_error,
> +                              IOS_DEV_IF_DATA (io->dev_if));
>    if (iod_error || io->dev == NULL)
>      goto error;
>  
>    /* Increment the id counter after all possible errors are avoided.  */
> -  io->id = ios_next_id++;
> +  io->id = ios_ctx.next_id++;
>  
>    /* Add the newly created space to the list, and update the current
>       space.  */
> -  io->next = io_list;
> -  io_list = io;
> +  io->next = ios_ctx.io_list;
> +  ios_ctx.io_list = io;
>  
> -  if (!cur_io || set_cur == 1)
> -    cur_io = io;
> +  if (!ios_ctx.cur_io || set_cur == 1)
> +    ios_ctx.cur_io = io;
>  
>    return io->id;
>  
> @@ -230,19 +236,20 @@ ios_close (ios io)
>    ret = io->dev_if->close (io->dev);
>  
>    /* Unlink the IOS from the list.  */
> -  assert (io_list != NULL); /* The list contains at least this IO space.  */
> -  if (io_list == io)
> -    io_list = io_list->next;
> +  /* The list contains at least this IO space.  */
> +  assert (ios_ctx.io_list != NULL);
> +  if (ios_ctx.io_list == io)
> +    ios_ctx.io_list = ios_ctx.io_list->next;
>    else
>      {
> -      for (tmp = io_list; tmp->next != io; tmp = tmp->next)
> +      for (tmp = ios_ctx.io_list; tmp->next != io; tmp = tmp->next)
>          ;
>        tmp->next = io->next;
>      }
>  
>    /* Set the new current IO.  */
> -  if (io == cur_io)
> -    cur_io = io_list;
> +  if (io == ios_ctx.cur_io)
> +    ios_ctx.cur_io = ios_ctx.io_list;
>  
>    /* Invalidate the sub-devices.  */
>    {
> @@ -274,13 +281,13 @@ ios_handler (ios io)
>  ios
>  ios_cur (void)
>  {
> -  return cur_io;
> +  return ios_ctx.cur_io;
>  }
>  
>  void
>  ios_set_cur (ios io)
>  {
> -  cur_io = io;
> +  ios_ctx.cur_io = io;
>  }
>  
>  ios
> @@ -288,7 +295,7 @@ ios_search (const char *handler)
>  {
>    ios io;
>  
> -  for (io = io_list; io; io = io->next)
> +  for (io = ios_ctx.io_list; io; io = io->next)
>      if (STREQ (io->handler, handler))
>        break;
>  
> @@ -300,7 +307,7 @@ ios_search_by_id (int id)
>  {
>    ios io;
>  
> -  for (io = io_list; io; io = io->next)
> +  for (io = ios_ctx.io_list; io; io = io->next)
>      if (io->id == id)
>        break;
>  
> @@ -334,7 +341,7 @@ ios_set_bias (ios io, ios_off bias)
>  ios
>  ios_begin (void)
>  {
> -  return io_list;
> +  return ios_ctx.io_list;
>  }
>  
>  bool
> @@ -355,7 +362,7 @@ ios_map (ios_map_fn cb, void *data)
>    ios io;
>    ios io_next;
>  
> -  for (io = io_list; io; io = io_next)
> +  for (io = ios_ctx.io_list; io; io = io_next)
>      {
>        /* Note that the handler may close IO.  */
>        io_next = io->next;
> @@ -1648,12 +1655,13 @@ ios_foreign_iod (void)
>  }
>  
>  int
> -ios_register_foreign_iod (struct ios_dev_if *iod_if)
> +ios_register_foreign_iod (struct ios_dev_if *iod_if, void *data)
>  {
>    if (ios_dev_ifs[0] != NULL)
>      return IOS_ERROR;
>  
>    ios_dev_ifs[0] = iod_if;
> +  ios_ctx.foreign_dev_if_data = data;
>    return IOS_OK;
>  }
>  
> diff --git a/libpoke/ios.h b/libpoke/ios.h
> index 478cf030..dda3b737 100644
> --- a/libpoke/ios.h
> +++ b/libpoke/ios.h
> @@ -364,7 +364,7 @@ struct ios_dev_if *ios_foreign_iod (void);
>     Return IOS_OK otherwise.  */
>  
>  struct ios_dev_if;
> -int ios_register_foreign_iod (struct ios_dev_if *iod_if);
> +int ios_register_foreign_iod (struct ios_dev_if *iod_if, void *data);
>  
>  /* **************** Sub IO space **************** */
>  
> diff --git a/libpoke/libpoke.c b/libpoke/libpoke.c
> index f169becb..69c511a3 100644
> --- a/libpoke/libpoke.c
> +++ b/libpoke/libpoke.c
> @@ -1097,9 +1097,8 @@ pk_register_iod (pk_compiler pkc, struct pk_iod_if 
> *iod_if)
>    CF (get_flags);
>    CF (size);
>    CF (flush);
> -  CF (data);
>  #undef CF
>  
> -  (void) ios_register_foreign_iod (&foreign_iod_if);
> +  (void) ios_register_foreign_iod (&foreign_iod_if, iod_if->data);
>    return pkc->status;
>  }
> -- 
> 2.37.1
> 
> 
>