Re: [PATCH v2 2/2] libpoke, ios, poke: add flags param to pk_ios_search

[Jose E. Marchesi]

OK for master.
Thanks!

> This commit adds a flag to `pk_ios_search' function which enables
> the user to select between EXACT or PARTIAL matching of the handler
> while searching for the IOS.
>
> Also fixes the invocation of this function in poke program.
>
> 2023-11-01  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * libpoke/libpoke.h (pk_ios_search): Add new param: FLAGS to support
>       partial match of handlers.
>       * libpoke/libpoke.c (pk_ios_search): Likewise.
>       * libpoke/ios.h (ios_search): Likewise.
>       * libpoke/ios.c (ios_search): Likewise.
>       * poke/pk-ios.c (pk_ios_alien_token_handler): Update to do partial
>       search for IOS handlers.
>       * poke/pk-cmd-ios.c (pk_cmd_file): Call `pk_ios_search' with
>       `PK_IOS_SEARCH_F_EXACT' flag.
>       (pk_cmd_mem): Likewise.
>       (pk_cmd_nbd): Likewise.
>       * testsuite/poke.libpoke/api.c (STREQ): New macro.
>       (test_pk_ios): New function for testing IO space functions.
>       (main): Call `test_pk_ios'.
> ---
>
> Hi Jose.
>
> On Wed, Nov 01, 2023 at 12:44:49AM +0100, Jose E. Marchesi wrote:
>> >
>> > What about adding an additional argument to pk_ios_search instead of
>> > adding a new service?  Something like:
>> >
>> >    pk_ios pk_ios_search (pk_compiler pkc, const char *handler,
>> >                          int partial_p);
>> >
>> > We will have to bump the DSO version in the next release anyway.
>> >
>> > If we add a new service, I expect the following idiom to be used to
>> > support partial searches (like in pk_ios_ailen_token_handler):
>> >    
>> >    ios = pk_ios_search (poke_compiler, HANDLER);
>> >    if (ios == NULL)
>> >      ios = pkl_ios_search_partial (poke_compiler, HANDLER);
>> >
>> > which cannot be implemented as efficiently as this could:
>> >
>> >    ios = pk_ios_search (poke_compiler, HANDLER, 1 /* partial_p */);
>> 
>> Also, introducing that extra argument to pk_ios_search now will allow us
>> to turn it from a boolean to up to 32 flags in the future without
>> breaking binary compatibility, perhaps to select which heuristic to use
>> when allowing partial matches, or other flags altering the search
>> behavior.
>> 
>
> I like the idea!
> But I decided to call it flags explicitly (like other parts of the libpoke.h).
>
>
> Regards,
> Mohammad-Reza
>
>
>  ChangeLog                    | 17 +++++++
>  libpoke/ios.c                | 27 +++++++++--
>  libpoke/ios.h                | 12 +++--
>  libpoke/libpoke.c            |  4 +-
>  libpoke/libpoke.h            | 13 ++++--
>  poke/pk-cmd-ios.c            |  9 ++--
>  poke/pk-ios.c                |  2 +-
>  testsuite/poke.libpoke/api.c | 90 ++++++++++++++++++++++++++++++++++++
>  8 files changed, 157 insertions(+), 17 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 7d3c92fc..a22562da 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,20 @@
> +2023-11-01  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * libpoke/libpoke.h (pk_ios_search): Add new param: FLAGS to support
> +     partial match of handlers.
> +     * libpoke/libpoke.c (pk_ios_search): Likewise.
> +     * libpoke/ios.h (ios_search): Likewise.
> +     * libpoke/ios.c (ios_search): Likewise.
> +     * poke/pk-ios.c (pk_ios_alien_token_handler): Update to do partial
> +     search for IOS handlers.
> +     * poke/pk-cmd-ios.c (pk_cmd_file): Call `pk_ios_search' with
> +     `PK_IOS_SEARCH_F_EXACT' flag.
> +     (pk_cmd_mem): Likewise.
> +     (pk_cmd_nbd): Likewise.
> +     * testsuite/poke.libpoke/api.c (STREQ): New macro.
> +     (test_pk_ios): New function for testing IO space functions.
> +     (main): Call `test_pk_ios'.
> +
>  2023-11-01  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>  
>       * poke/pk-ios.c (pk_ios_alien_token_handler): Fix handling of
> diff --git a/libpoke/ios.c b/libpoke/ios.c
> index 974aface..93e2f780 100644
> --- a/libpoke/ios.c
> +++ b/libpoke/ios.c
> @@ -331,13 +331,32 @@ ios_zombie_p (ios io)
>  }
>  
>  ios
> -ios_search (ios_context ios_ctx, const char *handler)
> +ios_search (ios_context ios_ctx, const char *handler, uint32_t flags)
>  {
>    ios io;
> +  int exact_p = (flags & 1) == IOS_SEARCH_F_EXACT;
>  
> -  for (io = ios_ctx->io_list; io; io = io->next)
> -    if (STREQ (io->handler, handler))
> -      break;
> +  if (exact_p)
> +    {
> +      for (io = ios_ctx->io_list; io; io = io->next)
> +        if (STREQ (io->handler, handler))
> +          break;
> +    }
> +  else
> +    {
> +      ios ios_matched = NULL;
> +      size_t n_matched = 0;
> +
> +      for (io = ios_ctx->io_list; io; io = io->next)
> +        if (strstr (io->handler, handler) != NULL)
> +          {
> +            if (++n_matched > 1)
> +              return NULL;
> +            ios_matched = io;
> +          }
> +
> +      io = ios_matched;
> +    }
>  
>    return io;
>  }
> diff --git a/libpoke/ios.h b/libpoke/ios.h
> index 3c695bc4..07fdf8d4 100644
> --- a/libpoke/ios.h
> +++ b/libpoke/ios.h
> @@ -188,10 +188,16 @@ ios ios_cur (ios_context ios_ctx);
>  
>  void ios_set_cur (ios_context ios_ctx, ios io);
>  
> -/* Return the IO space operating the given HANDLER.  Return NULL if no
> -   such space exists.  */
> +/* Return the IO space operating the given HANDLER.  Based on the FLAGS
> +   search use different strategies for matching the HANDLER.
> +   Return NULL if no such space exists.  */
>  
> -ios ios_search (ios_context ios_ctx, const char *handler);
> +/* Bit 0 choose exact/partial search strategies.  Other bits can change
> +   the interpretation of this bit in the future.  */
> +#define IOS_SEARCH_F_EXACT 0
> +#define IOS_SEARCH_F_PARTIAL 1
> +
> +ios ios_search (ios_context ios_ctx, const char *handler, uint32_t flags);
>  
>  /* Return the IO space having the given ID.  Return NULL if no such
>     space exists.  */
> diff --git a/libpoke/libpoke.c b/libpoke/libpoke.c
> index 57d28828..a0bc7ee9 100644
> --- a/libpoke/libpoke.c
> +++ b/libpoke/libpoke.c
> @@ -618,10 +618,10 @@ pk_ios_flags (pk_ios io)
>  }
>  
>  pk_ios
> -pk_ios_search (pk_compiler pkc, const char *handler)
> +pk_ios_search (pk_compiler pkc, const char *handler, uint32_t flags)
>  {
>    pkc->status = PK_OK;
> -  return (pk_ios) ios_search (pvm_ios_context (pkc->vm), handler);
> +  return (pk_ios) ios_search (pvm_ios_context (pkc->vm), handler, flags);
>  }
>  
>  pk_ios
> diff --git a/libpoke/libpoke.h b/libpoke/libpoke.h
> index 988df231..5699e705 100644
> --- a/libpoke/libpoke.h
> +++ b/libpoke/libpoke.h
> @@ -473,10 +473,17 @@ pk_ios pk_ios_cur (pk_compiler pkc) LIBPOKE_API;
>  
>  void pk_ios_set_cur (pk_compiler pkc, pk_ios ios) LIBPOKE_API;
>  
> -/* Return the IO space operating the given HANDLER.  Return NULL if no
> -   such space exist in the given Poke incremental compiler.  */
> +/* Return the IO space operating the given HANDLER.  Based on the FLAGS
> +   search use different strategies for matching the HANDLER.
>  
> -pk_ios pk_ios_search (pk_compiler pkc, const char *handler) LIBPOKE_API;
> +   Return NULL if no such space exist in the given Poke incremental
> +   compiler.  */
> +
> +#define PK_IOS_SEARCH_F_EXACT   0  /* Match the whole HANDLER string.  */
> +#define PK_IOS_SEARCH_F_PARTIAL 1  /* Match if HANDLER is a sub-string.  */
> +
> +pk_ios pk_ios_search (pk_compiler pkc, const char *handler,
> +                      uint32_t flags) LIBPOKE_API;
>  
>  /* Return the IO space having the given ID.  Return NULL if no such
>     space exist in the given Poke incremental compiler.  */
> diff --git a/poke/pk-cmd-ios.c b/poke/pk-cmd-ios.c
> index 39a28bd9..046741ee 100644
> --- a/poke/pk-cmd-ios.c
> +++ b/poke/pk-cmd-ios.c
> @@ -172,7 +172,7 @@ pk_cmd_file (int argc, struct pk_cmd_arg argv[], uint64_t 
> uflags)
>      /* /c has no effect if the file exists.  */
>      create_p = 0;
>  
> -  if (pk_ios_search (poke_compiler, filename) != NULL)
> +  if (pk_ios_search (poke_compiler, filename, PK_IOS_SEARCH_F_EXACT) != NULL)
>      {
>        printf (_("File %s already opened.  Use `.ios #N' to switch.\n"),
>                filename);
> @@ -467,7 +467,8 @@ pk_cmd_mem (int argc, struct pk_cmd_arg argv[], uint64_t 
> uflags)
>            if (asprintf (&mem_name, "*%d*", i) == -1)
>              pk_fatal (_("out of memory"));
>  
> -          if (pk_ios_search (poke_compiler, mem_name) == NULL)
> +          if (pk_ios_search (poke_compiler, mem_name, PK_IOS_SEARCH_F_EXACT)
> +              == NULL)
>              break;
>          }
>      }
> @@ -477,7 +478,7 @@ pk_cmd_mem (int argc, struct pk_cmd_arg argv[], uint64_t 
> uflags)
>          pk_fatal (_("out of memory"));
>      }
>  
> -  if (pk_ios_search (poke_compiler, mem_name) != NULL)
> +  if (pk_ios_search (poke_compiler, mem_name, PK_IOS_SEARCH_F_EXACT) != NULL)
>      {
>        printf (_("Buffer %s already opened.  Use `.ios #N' to switch.\n"),
>                mem_name);
> @@ -514,7 +515,7 @@ pk_cmd_nbd (int argc, struct pk_cmd_arg argv[], uint64_t 
> uflags)
>    const char *arg_str = PK_CMD_ARG_STR (argv[1]);
>    char *nbd_name = xstrdup (arg_str);
>  
> -  if (pk_ios_search (poke_compiler, nbd_name) != NULL)
> +  if (pk_ios_search (poke_compiler, nbd_name, PK_IOS_SEARCH_F_EXACT) != NULL)
>      {
>        printf (_("Buffer %s already opened.  Use `.ios #N' to switch.\n"),
>                nbd_name);
> diff --git a/poke/pk-ios.c b/poke/pk-ios.c
> index a94604ad..80c7c826 100644
> --- a/poke/pk-ios.c
> +++ b/poke/pk-ios.c
> @@ -253,7 +253,7 @@ pk_ios_alien_token_handler (char delimiter,
>    memcpy (handler, id + 1, id_len - 2);
>    handler[id_len - 2] = '\0';
>  
> -  ios = pk_ios_search (poke_compiler, handler);
> +  ios = pk_ios_search (poke_compiler, handler, PK_IOS_SEARCH_F_PARTIAL);
>    if (ios)
>      {
>        /* The IO space alien token resolves to an int<32>.  */
> diff --git a/testsuite/poke.libpoke/api.c b/testsuite/poke.libpoke/api.c
> index 805c16e7..1c796fec 100644
> --- a/testsuite/poke.libpoke/api.c
> +++ b/testsuite/poke.libpoke/api.c
> @@ -31,6 +31,8 @@
>  
>  #include "term-if.h"
>  
> +#define STREQ(a, b) (strcmp ((a), (b)) == 0)
> +
>  #define T(name, cond)                                                        
>  \
>    do                                                                         
>  \
>      {                                                                        
>  \
> @@ -135,6 +137,93 @@ test_pk_load (pk_compiler pkc)
>    T ("pk_load_1 exception", exception == 0);
>  }
>  
> +static void
> +test_pk_ios (pk_compiler pkc)
> +{
> +#define N 4
> +  int ios_id[N];
> +  pk_ios ios[N];
> +  uint64_t flags = 0;
> +
> +  ios_id[0] = pk_ios_open (pkc, "*foo*", flags, /*set_cur_p*/ 1);
> +  T ("pk_ios_open_1", ios_id[0] == 0);
> +  ios[0] = pk_ios_cur (pkc);
> +  T ("pk_ios_cur_1", ios[0] != NULL);
> +
> +  ios_id[1] = pk_ios_open (pkc, "*foobar*", flags, /*set_cur_p*/ 0);
> +  T ("pk_ios_open_2", ios_id[1] == 1);
> +  T ("pk_ios_cur_2", ios[0] == pk_ios_cur (pkc));
> +
> +  ios_id[2] = pk_ios_open (pkc, "*funfoo*", flags, /*set_cur_p*/ 0);
> +  T ("pk_ios_open_3", ios_id[2] == 2);
> +  T ("pk_ios_cur_3", ios[0] == pk_ios_cur (pkc));
> +
> +  ios_id[3] = pk_ios_open (pkc, "*baz*", flags, /*set_cur_p*/ 0);
> +  T ("pk_ios_open_4", ios_id[3] == 3);
> +  T ("pk_ios_cur_4", ios[0] == pk_ios_cur (pkc));
> +
> +  ios[1] = pk_ios_search_by_id (pkc, ios_id[1]);
> +  ios[2] = pk_ios_search_by_id (pkc, ios_id[2]);
> +  ios[3] = pk_ios_search_by_id (pkc, ios_id[3]);
> +  T ("pk_ios_search_by_id_1", ios[1] != NULL);
> +  T ("pk_ios_search_by_id_2", ios[2] != NULL);
> +  T ("pk_ios_search_by_id_3", ios[3] != NULL);
> +
> +  T ("pk_ios_get_id_1", pk_ios_get_id (ios[0]) == ios_id[0]);
> +  T ("pk_ios_get_id_2", pk_ios_get_id (ios[1]) == ios_id[1]);
> +  T ("pk_ios_get_id_3", pk_ios_get_id (ios[2]) == ios_id[2]);
> +  T ("pk_ios_get_id_4", pk_ios_get_id (ios[3]) == ios_id[3]);
> +
> +  T ("pk_ios_handler_1", pk_ios_handler (ios[0]) != NULL);
> +  T ("pk_ios_handler_2", pk_ios_handler (ios[1]) != NULL);
> +  T ("pk_ios_handler_3", pk_ios_handler (ios[2]) != NULL);
> +  T ("pk_ios_handler_4", pk_ios_handler (ios[3]) != NULL);
> +
> +  T ("pk_ios_handler_5", STREQ (pk_ios_handler (ios[0]), "*foo*"));
> +  T ("pk_ios_handler_6", STREQ (pk_ios_handler (ios[1]), "*foobar*"));
> +  T ("pk_ios_handler_7", STREQ (pk_ios_handler (ios[2]), "*funfoo*"));
> +  T ("pk_ios_handler_8", STREQ (pk_ios_handler (ios[3]), "*baz*"));
> +
> +  T ("pk_ios_search_1",
> +     pk_ios_search (pkc, "/some/non-existent/thing", PK_IOS_SEARCH_F_PARTIAL)
> +         == NULL);
> +
> +  T ("pk_ios_search_2",
> +     pk_ios_search (pkc, "baz", PK_IOS_SEARCH_F_PARTIAL) == ios[3]);
> +
> +  T ("pk_ios_search_3",
> +     pk_ios_search (pkc, "baz*", PK_IOS_SEARCH_F_PARTIAL) == ios[3]);
> +
> +  T ("pk_ios_search_4",
> +     pk_ios_search (pkc, "*baz", PK_IOS_SEARCH_F_PARTIAL) == ios[3]);
> +
> +  T ("pk_ios_search_5",
> +     pk_ios_search (pkc, "*baz*", PK_IOS_SEARCH_F_PARTIAL) == ios[3]);
> +
> +  T ("pk_ios_search_6",
> +     pk_ios_search (pkc, "az", PK_IOS_SEARCH_F_PARTIAL) == ios[3]);
> +
> +  T ("pk_ios_search_7",
> +     pk_ios_search (pkc, "z", PK_IOS_SEARCH_F_PARTIAL) == ios[3]);
> +
> +  T ("pk_ios_search_8",
> +     pk_ios_search (pkc, "bar*", PK_IOS_SEARCH_F_PARTIAL) == ios[1]);
> +
> +  T ("pk_ios_search_9",
> +     pk_ios_search (pkc, "bar", PK_IOS_SEARCH_F_PARTIAL) == ios[1]);
> +
> +  T ("pk_ios_search_10",
> +     pk_ios_search (pkc, "foo", PK_IOS_SEARCH_F_PARTIAL) == NULL);
> +
> +  T ("pk_ios_search_11",
> +     pk_ios_search (pkc, "ba", PK_IOS_SEARCH_F_PARTIAL) == NULL);
> +
> +  for (int i = N; i != 0; --i)
> +    pk_ios_close (pkc, ios[i - 1]);
> +
> +#undef N
> +}
> +
>  int
>  main ()
>  {
> @@ -144,6 +233,7 @@ main ()
>  
>    test_pk_keyword_p (pkc);
>    test_pk_load (pkc);
> +  test_pk_ios (pkc);
>    test_pk_compiler_free (pkc);
>  
>    return 0;