Re: [PATCH] open: Normalize tricky .file names

[Jose E. Marchesi]

Hi Eric.

    Otherwise, we can run into:
    $ touch "*#1*"
    $ ./run poke
    (poke) .mem #0
    The current IOS is now `*#0*'.
    (poke) .file "*#1*"
    The current IOS is now `*#1*'.
    (poke) .mem #1
    Buffer *#1* already opened.  Use `.ios #N' to switch.
    
    * src/ios-dev.h (struct ios_dev_if): Replace handler_p with
    handler_noramlize callback.
    * src/ios.c (ios_open): Store normalized name per ios.
    * src/ios-dev-file.c (ios_dev_file_handler_p): Rename...
    (ios_dev_file_handler_normalize): ...and prepend ./ to risky
    relative file names.
    * src/ios-dev-mem.c (ios_dev_mem_handler_p): Rename...
    (ios_dev_mem_handler_normalize): ...to this.
    (ios_dev_mem_open, ios_dev_mem_close): Drop unused handler member.
    * testsuite/poke.cmd/file-relative.pk: New test.

This is OK for master.
Thanks!

    ---
     ChangeLog                           | 14 ++++++++++++++
     src/ios-dev-file.c                  | 23 ++++++++++++++++++-----
     src/ios-dev-mem.c                   | 12 +++++-------
     src/ios-dev.h                       |  6 +++---
     src/ios.c                           |  4 ++--
     testsuite/poke.cmd/file-relative.pk |  7 +++++++
     6 files changed, 49 insertions(+), 17 deletions(-)
     create mode 100644 testsuite/poke.cmd/file-relative.pk
    
    diff --git a/ChangeLog b/ChangeLog
    index f8581455..52005806 100644
    --- a/ChangeLog
    +++ b/ChangeLog
    @@ -1,3 +1,17 @@
    +2020-02-26  Eric Blake  <address@hidden>
    +
    +   open: Normalize tricky .file names.
    +   * src/ios-dev.h (struct ios_dev_if): Replace handler_p with
    +   handler_noramlize callback.
    +   * src/ios.c (ios_open): Store normalized name per ios.
    +   * src/ios-dev-file.c (ios_dev_file_handler_p): Rename...
    +   (ios_dev_file_handler_normalize): ...and prepend ./ to risky
    +   relative file names.
    +   * src/ios-dev-mem.c (ios_dev_mem_handler_p): Rename...
    +   (ios_dev_mem_handler_normalize): ...to this.
    +   (ios_dev_mem_open, ios_dev_mem_close): Drop unused handler member.
    +   * testsuite/poke.cmd/file-relative.pk: New test.
    +
     2020-02-26  Eric Blake  <address@hidden>
    
        * src/ios.c (ios_close): Avoid side-effect inside assert.
    diff --git a/src/ios-dev-file.c b/src/ios-dev-file.c
    index 4003cf9c..31e833a5 100644
    --- a/src/ios-dev-file.c
    +++ b/src/ios-dev-file.c
    @@ -42,11 +42,24 @@ struct ios_dev_file
       uint64_t flags;
     };
    
    -static int
    -ios_dev_file_handler_p (const char *handler)
    +static char *
    +ios_dev_file_handler_normalize (const char *handler)
     {
    -  /* This backend is special, in the sense it accepts any handler.  */
    -  return 1;
    +  /* This backend is special, in the sense it accepts any handler.
    +     However, we want to ensure that the ios name is unambiguous from
    +     other ios devices, by prepending ./ to relative names that might
    +     otherwise be confusing.  */
    +  static const char safe[] =
    +    "abcdefghijklmnopqrstuvwxyz"
    +    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
    +    "0123456789/+_-";
    +  char *ret;
    +
    +  if (handler[0] == '/' || strspn (handler, safe) == strlen (handler))
    +    return xstrdup (handler);
    +  if (asprintf (&ret, "./%s", handler) == -1)
    +    assert (0);
    +  return ret;
     }
    
     static void *
    @@ -184,7 +197,7 @@ ios_dev_file_size (void *iod)
    
     struct ios_dev_if ios_dev_file =
       {
    -   .handler_p = ios_dev_file_handler_p,
    +   .handler_normalize = ios_dev_file_handler_normalize,
        .open = ios_dev_file_open,
        .close = ios_dev_file_close,
        .tell = ios_dev_file_tell,
    diff --git a/src/ios-dev-mem.c b/src/ios-dev-mem.c
    index 098a327f..52033c96 100644
    --- a/src/ios-dev-mem.c
    +++ b/src/ios-dev-mem.c
    @@ -28,7 +28,6 @@
    
     struct ios_dev_mem
     {
    -  char *handler;
       char *pointer;
       size_t size;
       size_t cur;
    @@ -38,10 +37,11 @@ struct ios_dev_mem
     #define MEM_STEP (512 * 8)
    
     static int
    -ios_dev_mem_handler_p (const char *handler)
    +ios_dev_mem_handler_normalize (const char *handler)
     {
    -  return (handler[0] == '*'
    -          && handler[strlen(handler) - 1] == '*');
    +  if (handler[0] == '*' && handler[strlen (handler) - 1] == '*')
    +    return xstrdup (handler);
    +  return NULL;
     }
    
     static void *
    @@ -50,7 +50,6 @@ ios_dev_mem_open (const char *handler, uint64_t flags, 
int *error)
       struct ios_dev_mem *mio;
    
       mio = xmalloc (sizeof (struct ios_dev_mem));
    -  mio->handler = xstrdup (handler);
       mio->pointer = xmalloc (MEM_STEP);
       mio->size = MEM_STEP;
       mio->cur = 0;
    @@ -64,7 +63,6 @@ ios_dev_mem_close (void *iod)
     {
       struct ios_dev_mem *mio = iod;
    
    -  free (mio->handler);
       free (mio->pointer);
       free (mio);
    
    @@ -139,7 +137,7 @@ ios_dev_mem_size (void *iod)
    
     struct ios_dev_if ios_dev_mem =
       {
    -   .handler_p = ios_dev_mem_handler_p,
    +   .handler_normalize = ios_dev_mem_handler_normalize,
        .open = ios_dev_mem_open,
        .close = ios_dev_mem_close,
        .tell = ios_dev_mem_tell,
    diff --git a/src/ios-dev.h b/src/ios-dev.h
    index 5bc5a257..1ed63008 100644
    --- a/src/ios-dev.h
    +++ b/src/ios-dev.h
    @@ -47,7 +47,7 @@ typedef uint64_t ios_dev_off;
     struct ios_dev_if
     {
       /* Determine whether the provided HANDLER is recognized as a valid
    -     device spec by this backend.  Return 1 if the handler is
    -     recognized, 0 otherwise.  */
    +     device spec by this backend, and if so, return its normalized
    +     form (caller will free).  If not, return NULL.  */
    
    -  int (*handler_p) (const char *handler);
    +  char *(*handler_normalize) (const char *handler);
    
       /* Open a device using the provided HANDLER.  Return the opened
          device, or NULL if there was an error.  In case of invalid flags,
    diff --git a/src/ios.c b/src/ios.c
    index 4bdc80d6..bb760161 100644
    --- a/src/ios.c
    +++ b/src/ios.c
    @@ -172,14 +172,14 @@ ios_open (const char *handler, uint64_t flags, int 
set_cur)
       io = xmalloc (sizeof (struct ios));
       io->id = ios_next_id++;
       io->next = NULL;
    -  io->handler = xstrdup (handler);
       io->bias = 0;
    
       /* Look for a device interface suitable to operate on the given
          handler.  */
       for (dev_if = ios_dev_ifs; *dev_if; ++dev_if)
         {
    -      if ((*dev_if)->handler_p (handler))
    +      io->handler = (*dev_if)->handler_normalize (handler);
    +      if (io->handler)
             break;
         }
    
    diff --git a/testsuite/poke.cmd/file-relative.pk 
b/testsuite/poke.cmd/file-relative.pk
    new file mode 100644
    index 00000000..82b3f2aa
    --- /dev/null
    +++ b/testsuite/poke.cmd/file-relative.pk
    @@ -0,0 +1,7 @@
    +/* { dg-do run } */
    +/* { dg-data {c*} {0x10 0x20 0x30 0x40 0x50 0x60 0x70 0x80} a#b } */
    +
    +/* { dg-command { .file "a#b" } } */
    +/* { dg-command { .info ios } } */
    +/* { dg-output "  Id\tMode\tPosition\tName" } */
    +/* { dg-output "\n\\* #0\trw\t0x00000000#b\t./a#b" } */