Mail Archives: cygwin-developers/2002/06/30/18:37:17
On Sun, Jun 30, 2002 at 11:23:09PM +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.
>I took a look at your changes and this still won't work. Look at where
>fill_filebuf is called in fhandler_virtual::lseek. The p member of
>fhandler_process must be valid at this point, but it is not because the code
>in fhandler_process::open sets it back to NULL after it has called
>fill_filebuf. The reason for calling fill_filebuf in lseek is that this is
>how the Linux proc utilities work - they open the file and then call seek
>(fd, 0, SEEK_SET) when they want the file contents updated.
Ok, but you can't keep the shared memory for every process open for the
duration of the life of a fhandler_process. I don't know how to deal with
this but using up lots of resources isn't the way to do it.
cgf
- Raw text -