+ if (rv == APR_SUCCESS) { + return newSViv((int)length); + } + + return &PL_sv_undef; +}
won't it be better to die if it fails and stick rv into [EMAIL PROTECTED] e.g. if you try to read a bucket which has no data? Just thinking aloud here, we have many places where don't quite handle error codes, but I tend to at least stick XXX in them and hopefully figure it out when we get first failures.
yeah, I wasn't quite sure what to do here. I didn't like the idea of length() returning an $rv and populating a $length argument, since it seemed more useful to simply have it return the length. but I'm deviating from the API here, so I dunno.
at least mark it with XXX, so we know it's an untied end. We will how to deal with it better as this API will be used.
as for sticking the rv in $@, I don't see that being useful, since it's just an APR_ status code.
But you can convert the code into the error message. We attempt to do that in several places. Very inconsistently though.
grep for modperl_apr_strerror and modperl_strerr
In the test script you end up allocating memory for 300k instead of 200k, just to throw 100k of it right away. a big waste.
hmm, the length passed ends up being allocated? I was just trying to simulate a user that used a big number since the size was unknown (and who didn't know about length())
ok then. I'd add at least a comment that one should pass either a small known size (if they want to get at most X bytes) or call the length method. some people use tests as examples, while we don't have the normal docs up.
I don't think flatten is effective, as it has to go through all the bucket twice - once because you need to figure out its length, second time to slurp data.
I was getting that feeling as I implemented it - the dependency on the length made it seem less than optimal for a "slurp" mode.
pflatten is more like 'slurp' and more suitable for Joe. though we already agreed that he doesn't need the *flatten perl glue, and better handle that on the C level with a slurp() wrapper as a perl glue.
More effecient and simpler for the end user (though requires the dreaded pool object)
yeah, that's the problem - requiring a pool is turning this all into less of a shortcut. but just because it takes more effort doesn't mean we shouldn't open them both up anyway.
it doesn't make it less a shortcut, since you usually do have a ready pool object. It's just the use of pool that makes me nervous. Obviously copying the string to protect from using the wrong pool is not an option here.
so, I'll do them both and whip up a patch for the Apache::Request docs that use pflatten() instead.
I think your current patch (plus a few comments) is fine to go in. Can do pflatten on the next iteration.
If Joe is going to implement slurp() I don't think A::R docs need to mention any flatten at all. Just wait a bit and the need will disappear ;)
__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
