Mailing-List: contact cygwin-developers-help AT sourceware DOT cygnus DOT com; run by ezmlm List-Subscribe: List-Archive: List-Post: List-Help: , 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 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