At Mon, 13 Jun 2005 08:09:08 -0400, Daniel Veillard wrote: > However I'm a bit uneasy with transforming casual return handling in case > of NULL parameters, sometimes it's just cumbersome to test NULL True. My own philosophy is to be conservative in this regard and only permit dubious parameters when actually necessary. That is, allow the caller program to pass NULL once a situation has been demonstrated where it is useful; not assume that a situation will manifest itself. > Applying your patch (please use mail attachment rather than including in > the mail) for example makes some of the regression tests just crash: > > make[1]: Entering directory `/u/veillard/gamin/tests' > [...] > running test 8 Okay, I found it. It appears the problem is in the gamin poll code. I missed it because I was testing on my port and we don't use the polling backend in the Hurd notify code. Apparently, dnotify does when a file/directory does not exist. The attached patch fixes the regression and make check passes. (Although I can't get the python test suite to run here for some reason...) The only caller of gam_poll_first_scan_dir is gam_poll_consume_subscriptions. In the case where the directory does not exist, _first_scan_dir would lookup the node corresponding to dpath (creating it if it did not exist) and add the subscription to it. This was unnecessary: dir_node is the node cooresponding to dpath and sub was added to dir_node immediately before _consume_subscriptions called _first_scan_dir. > I.e. adding assert without checking all calling points first for the > test trades immediate instability over long term code maintainance. > All code in server/ is internal code. It's not API code, so IMHO > stability is more important than API/code maintainance. I can appreciate this stance for stable code, however, in development code it seems a bit unusual. > Oh and documetnation updates are very very welcome ! If you revamp > some of it please use the Okay. > /** > * functionname: Just to be clean: you want to repeat the name of the function? > * @arg1: ... > * @argn: ... > * > * Description of the function > * > * Returns description of the return value if non void Shouldn't this be @returns ... > I don't know what do to with your current patch, there are good points > but I don't feel like commiting if it breaks make tests. Until either of us > get to fix the breakage, I don't think the asserts should be > commited. Okay now? Thanks, Neal
Attachment:
gam-poll.diff
Description: Binary data