Mailing-List: contact cygwin-developers-help AT cygwin DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-developers-subscribe AT cygwin DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin-developers/>
List-Post: <mailto:cygwin-developers AT cygwin DOT com>
List-Help: <mailto:cygwin-developers-help AT cygwin DOT com>, <http://sources.redhat.com/ml/#faqs>
Sender: cygwin-developers-owner AT cygwin DOT com
Delivered-To: mailing list cygwin-developers AT cygwin DOT com
Date: Sun, 30 Jun 2002 13:10:26 -0400
From: Christopher Faylor <cgf AT redhat DOT com>
To: cygwin-developers AT cygwin DOT com
Subject: Re: changes to fhandler_process.cc from 02/06/2002 should be reverted
Message-ID: <20020630171026.GB32201@redhat.com>
Reply-To: cygwin-developers AT cygwin DOT com
Mail-Followup-To: cygwin-developers AT cygwin DOT com
References: <00a801c22036$1a7456b0$0100a8c0 AT advent02>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <00a801c22036$1a7456b0$0100a8c0@advent02>
User-Agent: Mutt/1.3.23.1i

On Sun, Jun 30, 2002 at 02:00:26PM +0100, Chris January wrote:
>I've just seen this ChangeLog entry, Chris:
>
>2002-06-02  Christopher Faylor  <cgf AT redhat DOT com>
>
> Remove unneeded sigproc.h includes throughout.
> * fhandler.h (fhandler_proc::fill_filebuf): Take a pinfo argument.
> * fhandler_proc.cc (fhandler_proc::get_proc_fhandler): Simplify search
> for given pid.
> (fhandler_proc::readdir): Assume that pid exists if it shows up in the
> winpid list.
> * fhandler_process.cc (fhandler_process::open): Simplify search for
> given pid.  Call fill_filebuf with pinfo argument.
> (fhandler_process::fill_filebuf): Pass pinfo here and assume that it
> exists.
> * pinfo.h (pinfo::remember): Define differently if sigproc.h is not
> included.
>
>IMHO, these changes need to be reverted. fhandler_base::fill_filebuf is
>virtual. If you add the pinfo parameter to fhandler_process::fill_filebuf,
>then you are defining a new function, not overriding the one in
>fhandler_base. Hence, /proc semantics whereby the file contents are
>refreshed on an lseek are broken.

I'll certainly consider changes, but your previous method of searching
the whole process table for a given pid when there already is a method
available for directly getting to the pid itself was flawed.  You used
this technique throughout your proc code and I thought it demonstrated
an unfamiliarity with the way that the pinfo class was supposed to work,
so I fixed it.

I will put back the pinfo pointer in the fhandler_process class but I
don't think that the entire checkin evidenced by the ChangeLog above
needs to be reverted.

cgf