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] |
Mostly good news recently: 1) Chris' 51 line ChangeLog on 01-19 is his personal best since Sept 2000. When combined with the 31 line ChangeLog on 01-16, they exceed by far his record for the millennium (67 lines in Sept 2000). This is in a thread that has "Hmm. I have a slightly less intrusive idea for how to handle this. I'll check it in shortly." Sorry Chris, I can't stop chuckling. It relieves the frustration you mentioned, but no hard feelings. 2) The code contains nice features and seems to work, although I find it needlessly confusing and don't fully understand it. I attach a few changes (mostly deletions and reversions) that IMO simplify it significantly while keeping Chris's code partitioning: - "change_possible" is back to boolean instead of ternary values. - arrays are back to size MAX_ETC_FILES - no duplicated calls to test_file_change on Novell - other useless etc::file_changed and etc::init calls avoided. 3) For future reference, the way the modified code works can be explained relatively simply, to the point where I feel I could write a formal proof. a) Initially the pwdgrp::state is "uninitialized". This causes all internal and external get{pw,gr}XX functions to call read_etc_{passwd,group} and thus pwdgrp::load(filename). b) When pwdgrp::load is called in the uninitialized state, it calls etc::init, which allocates an "ix handle", records a pointer to the w32 path, and calls etc::file_changed, thus indirectly etc::dir_changed (with "change_possible" initialized to false). If the /etc handle is NULL, FindFirstFileNotification is called and the "change_possible" array is set to true. If the handle is not NULL, change_possible is already true. The current filetime is recorded and the "change_possible" entry of the file itself is set to false. load() then loads the file and changes the state to "loaded". c) Subsequent calls to get{pw,gr}XX invoke pwdgrp::isinitializing, which calls etc::file_changed if the state is "loaded". etc::file_changed first calls etc::dir_changed, which returns true if change_possible is true or if /etc is on Novell or if /etc has changed. In this last case the array change_possible is set to true to force time stamp checks on all files. If dir_changed returns true, the file time is checked and file_changed returns true, always resetting change_possible for the file to false. If file_changed returns true, isinitializing () changes the state to "initializing" (insuring that it returns the same value when called several times in a row, e.g. due to mutex checks). read_etc_yy and pwdgrp::load are called, loading the file and setting the state to "loaded" (without calling etc::init). Possible changes in the w32 path are recorded by load and will be taken into account in future calls to etc::file_changed. d) Following a fork in the loaded state, the first call (in the child) to dir_changed (from 3) with false change_possible will detect that the NO_COPY /etc handle has been reset to NULL. As in (2) it calls FindFirstFileNotification and sets the change_possible array to true, which will eventually trigger a check of the timestamps. Note that: -any change to /etc/ or a fork will trigger a timestamp check. -a timestamp check is only performed: a) Initially (to set the time) b) Following a change to /etc c) After a fork d) On Novell. 4) To finish off the work and leave the code in a newly minted state, I suggest the following cleanup (mostly deletions): - class pwdgrp Delete operator = - pwdgrp::load Delete "fh = NULL" and use a common CloseHandle, instead of duplicating it. Instead of having type bool just to produce an "it failed" message, use type void and put a debug_printf on lines that have "res = false". - etc::file_changed Remove the test (!fn[n]) because a) it is never true (pc will always return a pointer to its path) and b) even if it was true, NULL is legal in FindFirstFile. If you insist on keeping it, move the test to init(). - etc::test_file_changed Consider reverting the code back to etc::file_changed. It is only called from that very short function. - etc::change_possible For modularity move "change_possible[n] = 0;" from file_changed to dir_changed. Move definition of "change_possible" from etc class to a static in dir_changed, exactly as changed_h already is. Alternatively put the handle and the string "/etc" in the etc class, to make it applicable to any directory. - etc::last_modified Remove from etc class and declare static inside test_file_changed, see change_possible above. 5) The not so good news: apparently dir_changed() is not triggered by a mv or an rm (WinME). cp and file edits do trigger it. 2003/01/20 Pierre Humblet <pierre.humblet@ieee.org> * pwdgrp.h (pwdgrp::isinitializing): Do not change the state when it is uninitialized. * uinfo.cc (pwdfrp::load): Only call etc::init when the state is uninitialized. * path.h: (class etc): Revert the type of change_possible to bool and the array sizes to MAX_ETC_FILES. * path.cc: (etc::change_possible): Revert type to bool and size to MAX_ETC_FILES. (etc::fn): Revert size to MAX_ETC_FILES. (etc::last_modified): Ditto. (etc::init): Delete first argument. Return curr_ix as handle. Call file_changed() instead of test_file_changed(). (etc::test_file_change): Do not test for negative values of change_possible and do not set it to -res. (etc::dir_changed): When the handle is NULL, call memset instead of test_file_changed. When the handle is invalid, return true.
Attachment:
try.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |