RE: Z/OS Enhancement to gnu lib

[Igor Todorovski <itodorov@ca.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 6139 bytes --]

This was the symptom we experienced with findutils (guarding the close in gnulib fixed it):
find . -name "test"
The output is:
find: '.': EDC5113I Bad file descriptor.

https://github.com/ZOSOpenTools/findutilsport/issues/4

We’ll do some more testing to see if there’s a better solution. Yes, there’s a chance it will lead to a leak in file descriptors.

__e2a_l is documented here: https://www.ibm.com/docs/en/zos/2.2.0?topic=functions-e2a-l-convert-characters-from-ebcdic-ascii

Unfortunately, _IOCC_GPN is not documented, but is in the exposed header files:
/usr/include/termios.h:      #define _IOCC_GPN           17   /* LFS Get pathname         */

I’ll check with the OS team to see if it can be documented.

Thank you for making the improvements!

Thanks,
Igor

From: Bruno Haible <bruno@clisp.org>
Date: Monday, January 30, 2023 at 6:48 AM
To: bug-gnulib@gnu.org <bug-gnulib@gnu.org>, Igor Todorovski <itodorov@ca.ibm.com>
Cc: Mike Fulton <fultonm@ca.ibm.com>
Subject: [EXTERNAL] Re: Z/OS Enhancement to gnu lib
Hi,

> I was wondering if the following changes I made to findutils (to get it to function on z/OS) can be merged into gnulib.
>
> The first change (fdopendir.c) guards the close call. Otherwise we get a bad file descriptor on z/OS.

What's the problem with that "bad file descriptor" error? On POSIX system,
EBADF is the natural error code in this situation.

If close() on z/OS does not support this situation gracefully, we'll need
to override the close() function, like we did on native Windows, see
 <https://www.gnu.org/software/gnulib/manual/html_node/close.html >.

> I am not sure if this has any other consequences, but so far I haven’t seen any issues with findutils.

If fd is not invalid, it would lead to a file descriptor leak, no?

> The second change (openat-proc.c) adds a way to get the pathname when given a file descriptor as an input.

Thanks. This looks reasonable.

> diff --git a/gl/lib/openat-proc.c b/gl/lib/openat-proc.c
> index 3bacf7d..bb788fd 100644
> --- a/gl/lib/openat-proc.c
> +++ b/gl/lib/openat-proc.c
> @@ -28,6 +28,7 @@
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <termios.h>
> #include <unistd.h>
>  #ifdef __KLIBC__
> @@ -53,7 +54,27 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char const *file)
>        return buf;
>      }
> -#ifndef __KLIBC__
> +#ifdef __MVS__
> +  {
> +    char dir[_XOPEN_PATH_MAX];
> +    int rc = w_ioctl(fd, _IOCC_GPN, _XOPEN_PATH_MAX, dir);
> +    if (rc == 0) {
> +      __e2a_l(dir, _XOPEN_PATH_MAX);
> +    }
> +    size_t bufsize;
> +    dirlen = strlen (dir);
> +    bufsize = dirlen + 1 + strlen (file) + 1; /* 1 for '/', 1 for null */
> +    if (OPENAT_BUFFER_SIZE < bufsize)
> +      {
> +        result = malloc (bufsize);
> +        if (! result)
> +          return NULL;
> +      }
> +
> +    strcpy (result, dir);
> +    result[dirlen++] = '/';
> +  }
> +#elif !defined( __KLIBC__)
> # define PROC_SELF_FD_FORMAT "/proc/self/fd/%d/"
>    {
>      enum {
>

There are a number of things that can be improved from this patch:
  - The <termios.h> include should only be done on platforms that need it,
    since not all platforms have <termios.h>.
  - GNU coding style: placement of braces, space between function name and
    argument list.
  - Put special cases for special operating systems after the generic/Linux
    case. Simply as a convenience, so that the reader does not turn away
    before getting to the main code.
  - Avoid code duplication of 12 lines of code, if it can easily be avoided.
  - Use 'sizeof dir' instead of '_XOPEN_PATH_MAX', so that the dimension
    of that array needs to be stated only once. Useful for future maintenance.

I did these improvements. What I then take from your patch is only 3 lines
of code. This does not require a copyright assigment from you to the FSF
(cf. <https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html >).
Therefore I am incorporating this code directly.

Btw, do you have reference documentation available for _IOCC_GPN and __e2a_l?
It generally is more advisable to use documented facilities than undocumented
ones.


2023-01-30  Bruno Haible  <bruno@clisp.org>

        at-internal: Add support for z/OS.
        Reported and draft patch by Igor Todorovski <itodorov@ca.ibm.com>.
        * lib/openat-proc.c [z/OS]: Include <termios.h>.
        (openat_proc_name): For z/OS, use an approach similar to kLIBC, with
        3 lines of z/OS specific code by Igor Todorovski <itodorov@ca.ibm.com>.

diff --git a/lib/openat-proc.c b/lib/openat-proc.c
index 2a6a85f069..6419a8cf5f 100644
--- a/lib/openat-proc.c
+++ b/lib/openat-proc.c
@@ -30,9 +30,12 @@
 #include <string.h>
 #include <unistd.h>

-#ifdef __KLIBC__
+#ifdef __KLIBC__ /* OS/2 */
 # include <InnoTekLIBC/backend.h>
 #endif
+#ifdef __MVS__ /* z/OS */
+# include <termios.h>
+#endif

 #include "intprops.h"

@@ -53,7 +56,8 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char const *file)
       return buf;
     }

-#ifndef __KLIBC__
+#if !(defined __KLIBC__ || defined __MVS__)
+  /* Generic code for Linux, Solaris, and similar platforms.  */
 # define PROC_SELF_FD_FORMAT "/proc/self/fd/%d/"
   {
     enum {
@@ -107,14 +111,21 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char const *file)
         dirlen = sprintf (result, PROC_SELF_FD_FORMAT, fd);
       }
   }
-#else
+#else /* (defined __KLIBC__ || defined __MVS__), i.e. OS/2 or z/OS */
   /* OS/2 kLIBC provides a function to retrieve a path from a fd.  */
   {
-    char dir[_MAX_PATH];
     size_t bufsize;

+# ifdef __KLIBC__
+    char dir[_MAX_PATH];
     if (__libc_Back_ioFHToPath (fd, dir, sizeof dir))
       return NULL;
+# endif
+# ifdef __MVS__
+    char dir[_XOPEN_PATH_MAX];
+    if (w_ioctl (fd, _IOCC_GPN, sizeof dir, dir) == 0)
+      __e2a_l (dir, sizeof dir);
+# endif

     dirlen = strlen (dir);
     bufsize = dirlen + 1 + strlen (file) + 1; /* 1 for '/', 1 for null */



[-- Attachment #2: Type: text/html, Size: 12361 bytes --]