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
X-WM-Posted-At: avacado.atomice.net; Sun, 30 Jun 02 23:15:12 +0100
Message-ID: <018e01c22083$9a6f8680$0100a8c0@advent02>
From: "Chris January" <chris AT atomice DOT net>
To: <cygwin-developers AT cygwin DOT com>
References: <00a801c22036$1a7456b0$0100a8c0 AT advent02> <20020630171026 DOT GB32201 AT redhat DOT com>
Subject: Re: changes to fhandler_process.cc from 02/06/2002 should be reverted
Date: Sun, 30 Jun 2002 23:15:12 +0100
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
X-Priority: 3
X-MSMail-Priority: Normal
X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2600.0000

> >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.
The reason I searched the whole process table each time was to catch the
case when a process went away between a seek/read. However I agree there are
probably better ways of catching this.

>
> 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.
My main concern was that fill_filebuf is called from seek in order to
refresh the contents of the file when seek is called. Therefore subclasses
must override this function if they want the contents to be refreshed. By ad
ding the pinfo argument the signature was different and so the function
wasn't overriden. If you can tidy up the code without changing the
signature, that would be great.

Chris