----- Original Message ---- > From: Marvin Humphrey <[email protected]> > To: [email protected] > Sent: Tue, May 17, 2011 1:34:22 AM > Subject: Re: [lucy-dev] C TAP test harnesses > > On Fri, May 13, 2011 at 11:33:55AM -0700, Joe Schaefer wrote: > > > Whoa, if the apreq harness can work without APR that would be sweet. I > > > looked at at.c and I see that it makes heavy use of apr_snprintf(). Can > > > you fall back to sprintf() safely? > > > > Haven't looked closely, but I don't see a priori why the stdlib's formatting > > functions can't be used effectively here. > > > > > It could be dropped without much effort, or these features added to the > > > > existing Lucy C test apparatus. > > > > > > Would apreq accept patches removing the APR dependency? And is at.h a >public > > > API? Ideally, we would want to bundle those files and use them >verbatim. > > > Maintaining our own fork for the sake of Charmonizer's tests is less > > > desirable, unless we decide we want to go all out and publish a > > > supported >C > > > TAP library ourselves. > > > > Yes, of course that would be accepted/encouraged in apreq. (The at.[ch] >stuff > > is also designed to work within an httpd server C module (tho we don't > > make >use > > of it in apreq) where the TAP output is sent out over HTTP.) Getting rid of > > the apr dependency would be a welcome change, so long as we're careful to > > avoid leaks. > > I put in a few hours on this over the weekend. I made some "progress", but I > don't think any of that "progress" so far improves on the existing apreq >code. > > > Joe warned me in IRC that the hard part of this refactoring job was likely to > be the excision of the APR memory pool code -- and he was right. Memory > allocation under a pool system has a distinct usage pattern, and since the > standard library doesn't provide pools, it doesn't support that usage pattern. > With the stdlib.h heap allocation functions, ordinarily you have to pair each > malloc() with a free(). With a memory pool, you don't have to do that -- you > can overwrite allocated pointers, and you can mix and match pointers to > static/data-segment memory with pointers to pool memory: > > /* Start off by allocating some memory from the apr_pool_t. */ > thing->ptr = (char*)apr_palloc(pool, 1); > > /* Later, we might overwrite that pool pointer with another pool pointer. > * Note that we don't free() the existing pointer first. */ > if (foo != NULL) { > thing->ptr = apr_pstrdup(pool, foo); > } > > /* Later, we might overwrite that pointer to pool memory with a pointer >to > * some static data. */ > static char bar[] = "bar"; > thing->ptr = &bar; > > /* When the cleanup finally happens, it doesn't matter what's in > * thing->ptr, because the *pool* keeps track of what memory needs to be > * freed. We never call free() on individual pointers. */ > apr_pool_clear(pool); > > Additionally, the apreq harness takes advantage of the fact that you can > register pseudo-destructors that are triggered by apr_pool_clear() and get run > just before the pool memory gets deallocated. > > apr_pool_cleanup_register(p, q, report_local_cleanup, > report_local_cleanup); > > The localizer functionality in the apreq test harness depends on these > pseudo-destructors. (And unusually, it's the user's responsibility to invoke > apr_pool_clear() directly and cause the subtest cleanup as a side effect, as > opposed to invoking something like "AT_subtest_done()" which cleans up memory > as an implementation detail.) During this destructor, certain information gets > transfered from the child test harness object to the parent in a way which is > hard to track under a malloc/free memory model. It all works fine when you're > playing by the loose rules of the APR pool system, but things get complicated > when you try to apply the constraints imposed by malloc/free. > > I don't see how we can make the localizer feature work once APR is removed > without substantial rearchitecting. And there's a bunch of other stuff that > has to happen too before the harness is Lucy-ready, all of it churn for zero > positive impact or mildly negative impact from the apreq perspective, such as > replacing apr_snprintf with a fixed up version of _snprintf under MSVC or > eliminating APR status codes. > > It doesn't seem to me like it's in Lucy's interest to proceed further down > this path, nor in apreq's. I don't like the idea of going to apreq-dev and > saying, "Here a series of patches that make your code crappier. It's a > significant rearchitecting with a lot of churn, so it almost certainly > introduces new bugs that you're going to have to deal with. And I'm doing > this because I want to take a private implementation detail of your project > and turn it into a public API that you have to support. We'll be counting on > you!"
Two hours of hacking on your pull request and I've gotten at.[ch] to the point where the apr dependence is gone and the apreq tests (tweaked by about 10 LOC) all pass. There are some warnings issued under maintainer mode, but I probably will just ignore those and change apreq to use this de-apr'd code instead. > If the apreq community were to decide spontaneously to remove APR as a > dependency from at.c/at.h and start publishing it as a public API with full > backwards compatibility promises, that would be great. But it doesn't make > sense for apreq to accept these changes, and it doesn't make sense for Lucy >to > depend on apreq code when it isn't in apreq's interest to tailor it to Lucy's > needs. > > If we're going to put in this much effort making a test harness work for us, > we should either fork it and own the code, or we should be contributing to a > common project with an official public API. If we can get lucy using the code at http://github.com/joesuf4/at then that can be the upstream location for both projects. WDYT?
