Re: Regression (last snapshot)

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

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

On Jul 22 13:44, Ken Brown wrote:
> On 7/22/2019 8:23 AM, Ken Brown wrote:
> > On 7/20/2019 6:55 PM, Houder wrote:
> >> 64-@@ uname -a
> >> CYGWIN_NT-6.1 Seven 3.1.0s(0.339/5/3) 2019-07-12 15:28 x86_64 Cygwin
> >>
> >> 64-@@ ls -lL <(grep bash .bashrc)
> >> ls: /dev/fd/63: No such file or directory
> >> pr-------- 1 Henri None 0 Jul 21 00:41 /dev/fd/63
> > 
> > Thanks for the report.  This is probably caused by my new FIFO code.  I'm
> > looking into it.
> 
> Actually, a bisection shows that the regression is due to the following commit:
> 
> commit 2607639992f6600135532831c8357c10cb248821
> Author: Erik M. Bray <erik.m.bray@gmail.com>
> Date:   Wed Apr 10 17:05:22 2019 +0200
> 
>      Improve error handling in /proc/[pid]/ virtual files.
> 
>      * Changes error handling to allow /proc/[pid]/ virtual files to be
>        empty in some cases (in this case the file's formatter should return
>        -1 upon error, not 0).
> 
>      * Better error handling of /proc/[pid]/stat for zombie processes:
>        previously trying to open this file on zombie processes resulted
>        in an EINVAL being returned by open().  Now the file can be read,
>        and fields that can no longer be read are just zeroed.
> 
>      * Similarly for /proc/[pid]/statm for zombie processes.
> 
>      * Similarly for /proc/[pid]/maps for zombie processes (in this case the
>        file can be read but is zero-length, which is consistent with observed
>        behavior on Linux.
> 
> 
> Erik, can you take a look?

I have a hunch.  It's this change:

@@ -355,7 +355,7 @@ fhandler_process::fill_filebuf ()
        }
       else
        filesize = process_tab[fileid].format_func (p, filebuf);
-      return !filesize ? false : true;
+      return filesize < 0 ? false : true;
     }
   return false;
 }

The formatter for /proc/PID/fd, format_process_fd, returns *valid*
negative values.  But the above patch treats all negative values as
error now.

The fact that format_process_fd returns negative values has historical
reasons.  Negative values of type virtual_ftype_t are files, positive
values are directories.

One way to fix this is to change this to all positive values.  At a
first glance I don't see any check for an explicit negative
virtual_ftype_t value, especially not in the only consumer
path_conv::check in path.cc, and the simple numbers have long been
replaced with enum values.  At the same time, we should better drop
some outdated comments in the virtual file fhandler classes, e.g.

  /* Returns 0 if path doesn't exist, >0 if path is a directory,
     -1 if path is a file, -2 if it's a symlink.  */


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]