Mailing-List: contact cygwin-developers-help AT sourceware DOT cygnus DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-developers-subscribe AT sources DOT redhat DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin-developers/>
List-Post: <mailto:cygwin-developers AT sources DOT redhat DOT com>
List-Help: <mailto:cygwin-developers-help AT sources DOT redhat DOT com>, <http://sources.redhat.com/ml/#faqs>
Sender: cygwin-developers-owner AT sources DOT redhat DOT com
Delivered-To: mailing list cygwin-developers AT sources DOT redhat DOT com
Date: Sun, 9 Sep 2001 12:16:45 -0400
From: Christopher Faylor <cgf AT redhat DOT com>
To: cygwin-developers AT cygwin DOT com
Subject: Re: [RFA] A kinder, gentler check for /etc/{passwd,group} changes
Message-ID: <20010909121645.A8059@redhat.com>
Reply-To: cygwin-developers AT cygwin DOT com
Mail-Followup-To: cygwin-developers AT cygwin DOT com
References: <20010908225133 DOT A17336 AT redhat DOT com> <20010909152458 DOT D937 AT cygbert DOT vinschen DOT de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20010909152458.D937@cygbert.vinschen.de>
User-Agent: Mutt/1.3.21i

On Sun, Sep 09, 2001 at 03:24:58PM +0200, Corinna Vinschen wrote:
>On Sat, Sep 08, 2001 at 10:51:33PM -0400, Christopher Faylor wrote:
>> Here's what I did, based on the FindFirstFileChangeNotification ideas.
>> 
>> It seems to get performance back down to around 1.3.2 levels when
>> combined with a couple of other minor changes.
>> 
>> Now that I see the patch, I realize that the etc_changed function
>> probably belongs in miscfuncs.cc rather than in dcrt0.cc.
>
>IMO it belongs into the class init_cygheap.  I did it that way in
>my version of that patch.
>
>> The only thing I don't know is if the etc_changed function actually
>> does anything useful.  I don't have a useful test case for that but
>> I thought that Corinna might.
>
>It works.  A simple test is ssh.  Start sshd, login using your
>account.  Edit /etc/passwd to use tcsh as login shell instead of
>bash.  Try another login.
>
>Testing the /etc/group stuff isn't that easy.  The current Cygwin
>version doesn't support supplementary groups to add when the user
>context changes due to a NTCreateToken() call.  I plan to add it
>to 1.3.4 though.
>
>> I wonder if we could generalize the similar code in passwd.cc and grp.cc
>> into some kind of class for 1.3.4...
>
>See my below patch. The whole class stuff is now in a common
>header file `pwdgrp.h'.  There could be more common stuff later,
>perhaps.
>
>On Sun, Sep 09, 2001 at  1:14:27AM -0400, Christopher Faylor wrote:
>> >Huh? We _have_ to read the file once. I don't think that extracting the
>> >modification date from an openfile is a likely performance hit!.
>> 
>> Hmm.  I was referring to the way that Corinna did this originally.  She
>> used FindFirstFile to get the date.  I'm not sure why she didn't use   
>> GetFileInformationByHandle, or something, but you're right, this
>> wouldn't be a huge performance hit.
>
>I did that since opening a file and getting the modification date
>by GetFileInformationByHandle() is more costly than FindFirstFile().
>
>Think of the difference.  Getting the modification time when opened
>the file the first time would be cheap since the file handle is
>already open.  Getting the modification time on each file check
>by opening the file and calling GetFileInf...() isn't that cheap
>anymore.

Yes, but we are guaranteed to be opening the file later on.  The
modification times could be stored then.  I think that's what Robert
is saying.

I agree that repeatedly opening and closing the file isn't required
when you're just checking to see if the file has been modified.  However,
when you are definitely going to be opening the file, I don't see any
reason to use the FindFirstFile method.

Anyway, this patch looks good.  Can you check it in?

cgf