{"thread":{"id":"11663","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","startedAt":"2008-01-18T04:27:08Z","lastAt":"2008-01-21T04:10:31Z","messageCount":13,"participants":["Linus Torvalds","Junio C Hamano","Johannes Sixt","Charles Bailey","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"65822","messageId":"alpine.LFD.1.00.0801172013270.2957@woody.linux-foundation.org","threadId":"11663","inReplyTo":"20080118035700.GA3458@spearce.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-18T04:27:08Z","receivedAt":"2008-01-18T04:27:08Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 17 Jan 2008, Shawn O. Pearce wrote:\n>\n> fast-import was relying on the fact that on most systems mmap() and\n> write() are synchronized by the filesystem's buffer cache. We were\n> relying on the ability to mmap() 20 bytes beyond the current end\n> of the file, then later fill in those bytes with a future write()\n> call, then read them through the previously obtained mmap() address.\n> \n> This isn't always true with some implementations of NFS, but it is\n> especially not true with our NO_MMAP=YesPlease build time option used\n> on some platforms.\n\nIn fact, even with mmap(), it's not guaranteed. There are really crappy \nmmap implementations out there, partly due to bad CPU design (virtual CPU \ncaches without coherency), but more often due to total crap OS.\n\n(Yeah, Linux did count in that area at some point. Long ago. Early 90's. \nMaybe)\n\nI think HP-UX used to have non-coherent mmap for the longest time, due to \ncarrying around some totally crap memory management based on some ancient \nBSD version that everybody else (including the BSD's) had long since \njettisoned.\n\nThat said, I suspect any unix you can run today (without calling it a \nretro setup) probably has coherent-enough mmap. The possible virtual cache \ncoherency issue is unlikely to be able to trigger this (and not relevant \non any sane hardware anyway).\n\n\t\t\t\tLinus\n"},{"id":"65824","messageId":"7vtzlbirz0.fsf@gitster.siamese.dyndns.org","threadId":"11663","inReplyTo":"20080118035700.GA3458@spearce.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-18T06:10:43Z","receivedAt":"2008-01-18T06:10:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> +extern void close_pack_windows(struct packed_git *, int);\n\nNobody seems to pass anything but true in retain_fd parameter.\nIs it worth it?\n\n> +void close_pack_windows(struct packed_git *p, int retain_fd)\n> +{\n> +\tstruct pack_window *tail_var = NULL, *n = p->windows;\n> +\tstruct pack_window **tail = &tail_var;\n> +\twhile (n) {\n> +\t\tstruct pack_window *w = p->windows;\n> +\n> +\t\tif (w->inuse_cnt) {\n> +\t\t\t*tail = w;\n> +\t\t\ttail = &w->next;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tmunmap(w->base, w->len);\n> +\t\tpack_mapped -= w->len;\n> +\t\tpack_open_windows--;\n> +\t\tn = w->next;\n> +\t\tfree(w);\n> +\t}\n> +\n> +\tp->windows = tail_var;\n> +\tif (p->windows)\n> +\t\twarning(\"pack windows still in-use during close attempt\");\n> +\telse if (!retain_fd && p->pack_fd != -1) {\n> +\t\tclose(p->pack_fd);\n> +\t\tp->pack_fd = -1;\n> +\t}\n> +}\n\nI am not sure about this inuse_cnt business.\n\nThe only caller we know that needs this function is fast-import\nthat wants to drop all windows into a pack while keeping the\nfile descriptor and it should not have an in-use windows.\n\nIt is unclear what semantics is the right one for callers that\ndo want to retain some windows but still want to call this\nfunction. If somebody is in the middle of chasing a delta chain\nand still calls this function, *why* is the call being made, and\nwhat is the right behaviour if not all the windows can be closed\nbecause of these open windows?\n\nWhat about the case the value passed in ratain_fd is 0, which\npresumably means the caller is asking us to close the file\ndescriptor? If we close the packfile, later accesses through\nthe unclosed windows will obviously barf and I understand that\nis why you are ignoring retain_fd in that case, but maybe for\nthe caller it was of higher priority that the packfile to get\nclosed than the remaining windows to be still usable. Or maybe\nthe caller wants to be notified of such an error, in which case\nit probably is not enough to just call warning().\n\nIOW, I think the patch is trying to be too flexible without\nhaving a clear definition of what that flexibility is trying to\nachieve.\n\nMaybe we would need more flexible version later, but I am not\nconvinced if the semantics the above patch implements is the\nright one.\n\nSo I'd prefer something much simpler like this one instead...\n\nvoid close_pack_windows(struct packed_git *p)\n{\n\twhile (p->windows) {\n\t\tstruct pack_window *w = p->windows;\n\n\t\tif (w->inuse_cnt)\n\t\t\tdie(\"pack '%s' still has outstanding windows\",\n\t\t\t\tp->pack_name)\n\t\tmunmap(w->base, w->len);\n\t\tpack_mapped -= w->len;\n\t\tpack_open_windows--;\n\t\tp->windows = w->next;\n\t\tfree(w);\n\t}\n}\n"},{"id":"65829","messageId":"47905B08.4040302@viscovery.net","threadId":"11663","inReplyTo":"20080118035700.GA3458@spearce.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-01-18T07:53:44Z","receivedAt":"2008-01-18T07:53:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Shawn O. Pearce schrieb:\n> This should do the trick. I SOB'd the patch as I did test it with\n> NO_MMAP and I think its valid. :-)\n\nThanks a lot. It works here on Windows, too.\n\n-- Hannes\n"},{"id":"65835","messageId":"20080118084201.GA14763@hashpling.org","threadId":"11663","inReplyTo":"alpine.LFD.1.00.0801172013270.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2008-01-18T08:42:01Z","receivedAt":"2008-01-18T08:42:01Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Thu, Jan 17, 2008 at 08:27:08PM -0800, Linus Torvalds wrote:\n> \n> \n> On Thu, 17 Jan 2008, Shawn O. Pearce wrote:\n> >\n> > fast-import was relying on the fact that on most systems mmap() and\n> > write() are synchronized by the filesystem's buffer cache. We were\n> > relying on the ability to mmap() 20 bytes beyond the current end\n> > of the file, then later fill in those bytes with a future write()\n> > call, then read them through the previously obtained mmap() address.\n> > \n> > This isn't always true with some implementations of NFS, but it is\n> > especially not true with our NO_MMAP=YesPlease build time option used\n> > on some platforms.\n> \n> In fact, even with mmap(), it's not guaranteed. There are really crappy \n> mmap implementations out there, partly due to bad CPU design (virtual CPU \n> caches without coherency), but more often due to total crap OS.\n> \n> (Yeah, Linux did count in that area at some point. Long ago. Early 90's. \n> Maybe)\n> \n> I think HP-UX used to have non-coherent mmap for the longest time, due to \n> carrying around some totally crap memory management based on some ancient \n> BSD version that everybody else (including the BSD's) had long since \n> jettisoned.\n> \n> That said, I suspect any unix you can run today (without calling it a \n> retro setup) probably has coherent-enough mmap. The possible virtual cache \n> coherency issue is unlikely to be able to trigger this (and not relevant \n> on any sane hardware anyway).\n> \n> \t\t\t\tLinus\n\nI've just checked the Mac OS X build and it looks like there is a mmap\nand git is indeed using it, so this is obviously an example of a\n\"really crappy\" mmap implementation.\n\nThis adds more ammunition to the fight against the whole Mac OS X is\npowered/built/based on UNIX myth.\n\nCharles.\n"},{"id":"65839","messageId":"20080118092652.GA22052@hashpling.org","threadId":"11663","inReplyTo":"20080118035700.GA3458@spearce.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2008-01-18T09:26:52Z","receivedAt":"2008-01-18T09:26:52Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"I can ACK this as fixing the test suite failures on Mac OS X 10.4.11\nboth with and without NO_MMAP=Yes on top of 1.5.4.rc3.24.gb53139 and\nalso on my oldish Fedora install with NO_MMAP=Yes.\n\nAcked-by: Charles Bailey \n"},{"id":"65840","messageId":"7v8x2nh3uw.fsf@gitster.siamese.dyndns.org","threadId":"11663","inReplyTo":"20080118092652.GA22052@hashpling.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-18T09:36:55Z","receivedAt":"2008-01-18T09:36:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey writes:\n\n> I can ACK this as fixing the test suite failures on Mac OS X 10.4.11\n> both with and without NO_MMAP=Yes on top of 1.5.4.rc3.24.gb53139 and\n> also on my oldish Fedora install with NO_MMAP=Yes.\n>\n> Acked-by: Charles Bailey \n\nWell, I do not want to be picky with these things, but you do\nnot own that area, so that would be \"Tested-by:\"\n\nI've already committed the simplified version I suggested to\nShawn.\n"},{"id":"65843","messageId":"20080118094553.GB22052@hashpling.org","threadId":"11663","inReplyTo":"7v8x2nh3uw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2008-01-18T09:45:53Z","receivedAt":"2008-01-18T09:45:53Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Fri, Jan 18, 2008 at 01:36:55AM -0800, Junio C Hamano wrote:\n> \n> Well, I do not want to be picky with these things, but you do\n> not own that area, so that would be \"Tested-by:\"\n> \n> I've already committed the simplified version I suggested to\n> Shawn.\n> \n\nSorry, I thought that Acked-by was an acceptable tag for people not\ninvolved in the development path of a patch. I don't want to add a\n\"Tested-by\" as I am not a fast-import user and I am *only* reporting\nthe success of the test suite, not any real-world testing.\n\nCharles.\n"},{"id":"65855","messageId":"7v8x2nflkj.fsf@gitster.siamese.dyndns.org","threadId":"11663","inReplyTo":"20080118094553.GB22052@hashpling.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-18T10:57:16Z","receivedAt":"2008-01-18T10:57:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey writes:\n\n> Sorry, I thought that Acked-by was an acceptable tag for people not\n> involved in the development path of a patch. I don't want to add a\n> \"Tested-by\" as I am not a fast-import user and I am *only* reporting\n> the success of the test suite, not any real-world testing.\n\nIt still counts as \"Tested-by:\". Earlier you saw breakage and\nnow you see it fixed in your environment, to which neither I nor\nShawn have access to. That's a good verification that the patch\nfixed the issue for you and your feedback was very much\nappreciated (your timely initial breakage report even more so).\n\nAcked-by: is usually given by people who are the most familiar\nwith the code being affected (or at least by the ones more\nfamiliar than the patch's author), in order to let me and the\nworld know that he thinks the contents of the patch makes sense.\nIOW, the patch was checked by somebody who knows the existing\ncode well enough to catch any subtleties in the existing code\nthat might have been broken if the patch's author was not\ncareful. It just felt slightly funny that anybody is giving an\nAcked-by: to a patch Shawn made to fast-import.c, which is his\nbrainchild ;-).\n\nSigned-off-by is about the kosherness from software license\npoint of view, while Acked-by and Tested-by are about the\ncorrectness.\n"},{"id":"65896","messageId":"alpine.LFD.1.00.0801180842350.2957@woody.linux-foundation.org","threadId":"11663","inReplyTo":"20080118084201.GA14763@hashpling.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-18T17:08:23Z","receivedAt":"2008-01-18T17:08:23Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 18 Jan 2008, Charles Bailey wrote:\n> \n> I've just checked the Mac OS X build and it looks like there is a mmap\n> and git is indeed using it, so this is obviously an example of a\n> \"really crappy\" mmap implementation.\n\nLooking closer, this is not necessarily the case here.\n\nGit uses MAP_PRIVATE, because that whole pack-file mapping was really \n*meant* to map an existing read-only pack-file, and fast-import seems to \nreally be screwing with it.\n\nIt so happens that Linux has a particularly clean and streamlined VM, and \nif you do only reads to a MAP_PRIVATE mapping on a normal filesystem, \nyou'll always be as coherent as with MAP_SHARED because Linux will simply \nmap in the page cache pages directly.\n\nBut this is definitely not portable, and the git fast-import mmap window \nusage before Shawn's patch it was simply wrong.\n\nSo in this case, it really was git that was crap.\n\nIt just happened to work because the Linux mmap handling is just generally \npretty sane. It probably also worked fine on pretty much any other modern \nUNIX (ie Solaris).\n\nI'm not quite sure what OS X does to MAP_PRIVATE mappings, but if OS X is \nstill based on Mach (with FreeBSD just as a single-server on top), I \nsuspect that may be why it broke on OS X. The Mach VM is insanely complex \nand does really odd things.\n\nBut the fact is, without MAP_SHARED, you shouldn't expect things to be \ncoherent, even if they often will be (especially for PROT_READ).\n\nBtw, even with Shawn's patch, I wonder if the index_data usage is correct.\n\n\t\t\tLinus\n"},{"id":"65934","messageId":"7vwsq6a44w.fsf@gitster.siamese.dyndns.org","threadId":"11663","inReplyTo":"alpine.LFD.1.00.0801180842350.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-19T03:25:03Z","receivedAt":"2008-01-19T03:25:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Btw, even with Shawn's patch, I wonder if the index_data usage is correct.\n\nHmph.\n\ngfi uses data in a \"pack\" in two quite different ways.\n\nA new object is written to an unfinalized pack. Such a pack\nalready has \"struct packed_git\" allocated for it and a pointer\nto it is held in pack_data. As far as the core part of git\n(that is, sha1_file.c) is concerned, however, this pack does not\neven exist. It is still not part of packed_git list in\nsha1_file.c, and read_sha1_file() will not see objects in it, as\nno idx into the packfile exists yet. gfi has a table of objects\nin this pack and uses gfi_unpack_entry() function to retrieve\ndata from it.\n\nA packfile is finalized in end_packfile(). The pack header and\nfooter is recomputed, an idx file is written, and the pack is\nfinally registered. Before that time p->index_data is not even\nused for that pack (it is initialized with NULL).\n\nSo I do not think \"index_data usage\" can be incorrect, as there\nwon't be any index_data usage with unfinalized pack, and the\ncore part of git would not even have any mmap(2) (nor open fd)\ninto its idx file before it is finalized.\n\nBy the way, I was quite puzzled how the gfi_unpack_entry()\nfunction manages to work correctly when it has to read an object\nit deltified based on another object it wrote into the same\nunfinalized pack earlier. It knows where in the unfinalized\npack it wrote the object, so it can find from its own \"struct\nobject_entry\" the offset for the object, and calls\nunpack_entry() defined in the core to do the rest.\n\nHowever, most of the core does not really know about the other\nobjects in this half-built pack. If the object is a delta,\nunpack_delta_entry() needs to find the delta base. And it needs\nto do that without having the idx.\n\nThe trick (the code really needs a bit more documentation) is\nthat gfi never writes anything but OFS_DELTA. So the core, even\nthough it does not have the corresponding idx file, does not\nhave to look up the object (in fact it does not even know what\nobject to look up for the base, it only knows the offset).\n"},{"id":"65936","messageId":"alpine.LFD.1.00.0801181953130.2957@woody.linux-foundation.org","threadId":"11663","inReplyTo":"7vwsq6a44w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-19T03:55:27Z","receivedAt":"2008-01-19T03:55:27Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 18 Jan 2008, Junio C Hamano wrote:\n> \n> gfi uses data in a \"pack\" in two quite different ways.\n> \n> A new object is written to an unfinalized pack. Such a pack\n> already has \"struct packed_git\" allocated for it and a pointer\n> to it is held in pack_data. As far as the core part of git\n> (that is, sha1_file.c) is concerned, however, this pack does not\n> even exist. It is still not part of packed_git list in\n> sha1_file.c, and read_sha1_file() will not see objects in it, as\n> no idx into the packfile exists yet. gfi has a table of objects\n> in this pack and uses gfi_unpack_entry() function to retrieve\n> data from it.\n> \n> A packfile is finalized in end_packfile(). The pack header and\n> footer is recomputed, an idx file is written, and the pack is\n> finally registered. Before that time p->index_data is not even\n> used for that pack (it is initialized with NULL).\n\nOh, ok. I did try to grep for index_data, and didn't find anything that \nlooked bad, but the incestuous things that fast-import.c does just made me \nworry - but I was too lazy to really follow it all. It's one of those \nfiles I don't think I've ever had anything what-so-ever to do with.\n\n> So I do not think \"index_data usage\" can be incorrect, as there\n> won't be any index_data usage with unfinalized pack, and the\n> core part of git would not even have any mmap(2) (nor open fd)\n> into its idx file before it is finalized.\n\nIn that case, I think Shawn fixed it all, and we're all good, and it's not \njust hidden well enough that it \"just happens\" to work.\n\n\t\t\t\tLinus\n"},{"id":"66069","messageId":"20080121035755.GC24004@spearce.org","threadId":"11663","inReplyTo":"7vwsq6a44w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-01-21T03:57:56Z","receivedAt":"2008-01-21T03:57:56Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Linus Torvalds writes:\n> \n> > Btw, even with Shawn's patch, I wonder if the index_data usage is correct.\n...\n> So I do not think \"index_data usage\" can be incorrect, as there\n> won't be any index_data usage with unfinalized pack, and the\n> core part of git would not even have any mmap(2) (nor open fd)\n> into its idx file before it is finalized.\n\nCorrect.\n \n> By the way, I was quite puzzled how the gfi_unpack_entry()\n> function manages to work correctly when it has to read an object\n> it deltified based on another object it wrote into the same\n> unfinalized pack earlier. It knows where in the unfinalized\n> pack it wrote the object, so it can find from its own \"struct\n> object_entry\" the offset for the object, and calls\n> unpack_entry() defined in the core to do the rest.\n> \n> However, most of the core does not really know about the other\n> objects in this half-built pack. If the object is a delta,\n> unpack_delta_entry() needs to find the delta base. And it needs\n> to do that without having the idx.\n> \n> The trick (the code really needs a bit more documentation) is\n> that gfi never writes anything but OFS_DELTA. So the core, even\n> though it does not have the corresponding idx file, does not\n> have to look up the object (in fact it does not even know what\n> object to look up for the base, it only knows the offset).\n\nYup.\n\nOlder fast-imports (pre OFS_DELTA) had to replicate a good chunk of\nthe unpack_entry() logic directly inside of fast-import. But it\nlater occurred to me that OFS_DELTA simplifies the code and lets\nme reuse more of the existing sha1_file.c implementation.\n\nSo yea, fast-import only emits OFS_DELTA, and does so only so that it\ncan pull this delta-base-unpack trick on the core, without actually\ngiving the core the corresponding idx file first.\n\n-- \nShawn.\n"},{"id":"66071","messageId":"20080121041031.GF24004@spearce.org","threadId":"11663","inReplyTo":"7vtzlbirz0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-01-21T04:10:31Z","receivedAt":"2008-01-21T04:10:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> \n> > +extern void close_pack_windows(struct packed_git *, int);\n> \n> Nobody seems to pass anything but true in retain_fd parameter.\n> Is it worth it?\n\nHmm, well, I originally wrote something like what you fixed it\nto be, then thought that at some point in the future someone may\ntry to invoke the method and expect different semantics than it\nimplemented. :-)\n\nIn short I made the implementation I gave you way too complex.\n \n> So I'd prefer something much simpler like this one instead...\n> \n> void close_pack_windows(struct packed_git *p)\n> {\n> \twhile (p->windows) {\n> \t\tstruct pack_window *w = p->windows;\n> \n> \t\tif (w->inuse_cnt)\n> \t\t\tdie(\"pack '%s' still has outstanding windows\",\n> \t\t\t\tp->pack_name)\n> \t\tmunmap(w->base, w->len);\n> \t\tpack_mapped -= w->len;\n> \t\tpack_open_windows--;\n> \t\tp->windows = w->next;\n> \t\tfree(w);\n> \t}\n> }\n\nMe too. Thank you for the sanity check, and for fixing the\ncode to actually be somewhat sane.\n\n-- \nShawn.\n"}]}