Re: [PATCH 2/3] ios: Move file name normalization logic into ios-dev.h

[Jose E. Marchesi]

    On 7/12/20 11:32 AM, Jose E. Marchesi wrote:
    > Hi Ege.
    >
    >      +#define IOS_FILE_HANDLER_NORMALIZE(handler, newhandler)             
        \
    >      +{                                                                   
\
    >      +  /* File devices are special, in the sense that they accept 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/+_-";                                               
        \
    >      +                                                                    
\
    >      +  if (handler[0] == '/' || strspn (handler, safe) == strlen 
(handler))      \
    >      +    (newhandler) = strdup ((handler));                              
        \
    >      +                                                                    
\
    >      +  if (asprintf (&(newhandler), "./%s", (handler)) == -1)            
\
    >      +    (newhandler) = NULL;                                            
\
    >      +}
    >
    > Maybe it would be better to use a static inline function here?
    > Otherwise, please surround the macro body with do..while(0)
    >
    > Other than that, OK for master.
    > Thanks! :)
    
    I am not happy with the implicit declaration warnings due to asprintf,
    with including stdio.h here or adding pragmas.
    I am keeping it as the macro for now. In nearby-future, if
    ios-dev-stream and ios-dev-file end up sharing more code, we can 
    consider getting those two a new .h file exclusively.
    
    I added the do-while upon your request and pushed the changes.
    
I just saw that the patch introduces a regression:

FAIL: poke.cmd/file-mode.pk output pattern test

I just pushed the fix below to master, that fixes it:

diff --git a/ChangeLog b/ChangeLog
index a69683a8..f06c088a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2020-07-14  Jose E. Marchesi  <jemarch@gnu.org>
+
+       * libpoke/ios-dev.h (IOS_FILE_HANDLER_NORMALIZE): Fix logic.
+
 2020-07-14  Jose E. Marchesi  <jemarch@gnu.org>
 
        * libpoke/pkl-gen.c (pkl_gen_pr_map): Fix subpassing on the map
diff --git a/libpoke/ios-dev.h b/libpoke/ios-dev.h
index 6689a243..d2f42f0f 100644
--- a/libpoke/ios-dev.h
+++ b/libpoke/ios-dev.h
@@ -100,7 +100,6 @@ do {                                                        
                \
                                                                        \
   if (handler[0] == '/' || strspn (handler, safe) == strlen (handler)) \
     (newhandler) = strdup ((handler));                                 \
-                                                                       \
-  if (asprintf (&(newhandler), "./%s", (handler)) == -1)               \
+  else if (asprintf (&(newhandler), "./%s", (handler)) == -1)          \
     (newhandler) = NULL;                                               \
 } while (0)