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

[Jose E. Marchesi]

This is good for master, but only after we branch 3.0.
Thanks!

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