This is the mail archive of the cygwin-developers@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: changes to fhandler_process.cc from 02/06/2002 should be reverted


> >I've just seen this ChangeLog entry, Chris:
> >
> >2002-06-02  Christopher Faylor  <cgf@redhat.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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]