This is the mail archive of the cygwin-patches@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] |
Hello Chris, I like your code, just made a few changes. The explanations below are detailed and longer that the changes! In load(), CloseFile wasn't called on read error, and it was called before GetFileTime. It seems that declaring passwd_buf, group_buf etc NO_COPY means that there will be a memory leak on fork, as the malloced buffers they point to will be orphans. Thus I made them regular static variables and I reuse the existing buffers on fork (same logic as it was before). Thus the pwdgrp class cannot be NO_COPY either, nor etc::curr_ix. Reusing the buffers creates a problem because the file(s) may have been touched after the buffers were loaded in the parent, but before the fork. dir_changed in the child will not observe the event. Thus on forks it is necessary to check the file dates when the buffers are already loaded, but not if they are not. This is achieved by having the default sawchange as forcing a date comparison and passing an argument to etc::init to set sawchange if the compare is not needed. The meaning of sawchange is changed: sawchange[n] is false if a change has not been seen by n (seems logical). sawchange[n] is now set in dir_changed itself, and not in file_changed nor in set_last_modified(). Setting it in set_last_modified becomes unnecessary due to the way sawchange is initialized (also that created a tiny race condition, I think). That made set_last_modified a one line function. I decided to get rid of it and to store last_modified in the pwdgrp class itself, where it is easier to update it. By the same token I removed fn from etc, simply passing the filename in file_changed. In passwd.cc and grp.cc I reverted to the debug_printf giving the line count, it has served me well in the past. The return value of load was then unused, so I made it void, which simplified load a little. This passes my test... Pierre P.S.: I have mixed feelings about having dir_changed always returning true if FindFirstChangeNotification has failed. The etc_changed stuff has been broken until now and this has generated only one complaint AFAIK. Users seem quite willing to restart their daemons after editing passwd. If only Novell users (with /etc on a share drive) need to do it now, that will already be a big improvement. 2003/01/17 Pierre Humblet <pierre.humblet@ieee.org> * pwdgrp.h (class etc): Remove members fn, last_modified and set_last_modified. Change arguments of init and file changed. (class pwdgrp): Add member last_modified. (pwdgrp::isinitializing): White space. Add arguments to file_changed. (pwdgrp::load): Change type to void. Change second argument of etc::init. Close file on read error, but never before calling GetFileTime. Do not call etc::set_last_modified. * uinfo.cc: Remove definitions of etc::fn and etc::last_modified. Do not define etc::curr_ix NO_COPY. (etc::init): Use second argument to initialize sawchange. Do not initialize fn. (etc:::dir_changed): sawchange[n] is now false when a change has happened but has not been seen by n. Initialize res accordingly. On observing a change set the sawchange array to false. Always set sawchange[n] to true. (etc::file_changed): Use new arguments. Do not set sawchange. Do not verify fn, FindFirstFile accepts NULL pointers. (etc::set_last_modified): Delete. * passwd.cc: Do not define passwd_buf, curr_lines, maxlines and pr NO_COPY. (read_etc_passwd): Revert to old style debug_printf. * group.cc: Do not define group_buf, curr_lines, maxlines and gr NO_COPY. (read_etc_group): Revert to old style debug_printf.
Attachment:
pwgr.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |