Re: Four patches against 2.3.14
On Fri, Mar 27, 2009 at 10:07:29AM +, David Carter wrote: ctype.patch === Mostly automatic using find and sed, but I did go back and remove casts to (unsigned char) [correct but redundant], and (int) [typically incorrect if the source is signed char, probably put there to stop the compiler moaning]. Why did this touch sieve.h? It's generated by bison. Did you not work on a clean tree? I guess it doesn't matter too much, I'll be working on a clean tree anyway :)
Re: Four patches against 2.3.14
On Fri, 27 Mar 2009, Bron Gondwana wrote: Why did this touch sieve.h? It's generated by bison. Did you not work on a clean tree? 2.3.14 ships with a sieve/sieve.h I did remove a number of the .c files which are generated from Bison or Flex, but missed this one. -- David Carter Email: david.car...@ucs.cam.ac.uk University Computing Service,Phone: (01223) 334502 New Museums Site, Pembroke Street, Fax: (01223) 334679 Cambridge UK. CB2 3QH.
Re: Four patches against 2.3.14
On Fri, Mar 27, 2009 at 10:07:29AM +, David Carter wrote: robust_squatter.patch = Don't give up if index_me() throws an error. Not wildly happy about mboxlist_findall which can run for several days in any case. l-tail = l-tail-next = n; Is that _guaranteed_ to evaluate correctly? It looks like someone being a smartarse to avoid writing: l-tail-next = n; l-tail = l-tail-next; Or even: tail = l-tail; tail-next = n; l-tail = n; Which is ultra-extra clear about what you're doing. It took me a couple of seconds parsing to understand what you were doing with that line, and it scares me. If the compiler re-optimised the order in which it did the assignment and lookup, things would get really sad. (it's annoying re-inventing such a common wheel in the first place really. The bikeshed painter in me wonders why it's not a realloced array, but that's more accounting and not worth it for a one-shot piece of code) Bron.
Re: Four patches against 2.3.14
On Fri, Mar 27, 2009 at 10:07:29AM +, David Carter wrote: http://www-uxsup.csx.cam.ac.uk/~dpc22/cyrus/patches/2.3.14 Ok - I've loaded all 4 of these patches on top of my 'master' tree on github and called the branch 'davidcarter', then rebased the fastmaildev branch on top of it. That means that it will be included in my dev builds from now on :) I did change the linked list append into two lines in my copy. Assuming this all runs nicely in my testbed, I'll roll it out to the usual suspects (man, people are going to LOVE being on our store...) and if that's good then I'm happy to commit it to CVS. NOTE: there are three separate trees on github, and they all have to stay in sync :( Bloody CVS separate repostiories. I've used exactly the same commit messages (and falsified the author as David on the git commits!) for each set across the respositories. Bron.
Re: Four patches against 2.3.14
On Fri, Mar 27, 2009 at 11:38:15AM +, David Carter wrote: On Fri, 27 Mar 2009, Bron Gondwana wrote: Why did this touch sieve.h? It's generated by bison. Did you not work on a clean tree? 2.3.14 ships with a sieve/sieve.h I did remove a number of the .c files which are generated from Bison or Flex, but missed this one. Fair enough :) It's not in my git tree, so I won't be updating it anyway on my copy... Bron.
Re: Four patches against 2.3.14
On Fri, 27 Mar 2009, Bron Gondwana wrote: l-tail = l-tail-next = n; Is that _guaranteed_ to evaluate correctly? It looks like someone being a smartarse to avoid writing: l-tail-next = n; l-tail = l-tail-next; It's a fairly common idiom for managing linked lists in C. Or at least I thought that it was a fairly common idiom: the only other obvious incidences I can find in Cyrus are sync_support.c, make_md5.c and make_sha1.c, which are probably all down to me. Smartarse. '=' has right to left associativity (KR second edition, page 53), so: a = b = c evaluates as: a = (b = c) But if you are happier with two or three lines of code, then go for it. -- David Carter Email: david.car...@ucs.cam.ac.uk University Computing Service,Phone: (01223) 334502 New Museums Site, Pembroke Street, Fax: (01223) 334679 Cambridge UK. CB2 3QH.
Re: Four patches against 2.3.14
On Fri, Mar 27, 2009 at 12:25:38PM +, David Carter wrote: '=' has right to left associativity (KR second edition, page 53), so: a = b = c evaluates as: a = (b = c) But if you are happier with two or three lines of code, then go for it. Yeah, I think it's clearer that it's two separate operations. Besides, I've followed some of the stuff on LKML about various versions of GCC screwing up optimisations, and debuging that stuff sounds like it would give me ulcers! I tell you, debugging the issue with blocks of zeros appearing in our skiplist files on 64 bit machines sure did... thankfully Linus found the bug quickly, though he did say that reading via mmap and writing with fwrite rather than just using mmap for writes was probably up there in insane territory as well. Surprised it worked so well everywhere, he was. Speaking of smartarse - you forgot to include util.h in sieve/test.c. It's not part of the standard build, so easy not to notice - but our install script builds it, so I noticed when I went to build it on my test machine. I've rolled that into my tree and pushed back to github again. Git is nice :) Bron.
Re: Four patches against 2.3.14
On Fri, Mar 27, 2009 at 04:12:41PM +, David Carter wrote: On Sat, 28 Mar 2009, Bron Gondwana wrote: Speaking of smartarse - you forgot to include util.h in sieve/test.c. It's not part of the standard build, so easy not to notice - but our install script builds it, so I noticed when I went to build it on my test machine. Actually that's not the only case. I meant to run a cross check this morning to make sure that all of the files that had been patched #include'd util.h, but I got diverted. Sorry about that. Here's a second attempt: http://www-uxsup.csx.cam.ac.uk/~dpc22/cyrus/patches/2.3.14/ctype2.patch This adds the #include to a number of extra files. It also doesn't patch ./makedepend, ./syslog or ./doc/text/htmlstrip.c, which are standalones. Yeah, that's good :) Github is rebuilt on top of that... (basically I created a new branch forking off just before the old ctype patch called ctype2, then applied the patch, then used 'rebase -i' for the davidcarter branch on top of the ctype2 branch, and deleted the line for the old ctype branch. All good :) Bron.
