You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Stas Bekman <st...@stason.org> on 2003/10/20 05:46:39 UTC
Re: [apreq-2] libapreq_cgi and getenv
Randy Kobes wrote:
> Hi,
> I've been looking at libapreq_cgi, and am having some
> problems getting cookie data from within a cgi program
> (which I'll describe in a separate message). I don't know if
> this is related, but would there be any objection (or harm?)
> in using, within libapreq_cgi.c, the apr function
> apr_env_get(), rather than getenv(), as in the following:
+1, apr_env_get is better since it doesn't require running Perl ;) and we use
it in mp2. It's probably faster as well...
Though since you get a status anyway:
> s = apr_env_get(&value, "QUERY_STRING", ctx->pool);
it doesn't hurt to check for failure. Probabaly write a macro to avoid noise
(adopted from mp2):
+ #define APREQ_ASSERT(rc_run) do { \
+ apr_status_t rc = rc_run; \
+ if (rc != APR_SUCCESS) { \
+ ap_log_error(APLOG_MARK, APLOG_ERR, rc, NULL, "failed: "); \
+ } \
+ } while (0)
{
- return getenv("QUERY_STRING");
+ dCTX;
+ char *value;
+ APREQ_ASSERT(apr_env_get(&value, "QUERY_STRING", ctx->pool));
+ return value;
}
may be a better error message...
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [apreq-2] libapreq_cgi and getenv
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
> On Sun, 19 Oct 2003, Stas Bekman wrote:
>
>
>>Randy Kobes wrote:
>>
>>
>>>>it doesn't hurt to check for failure. Probabaly write a macro to avoid noise
>>>>(adopted from mp2):
>>>>
>>>>+ #define APREQ_ASSERT(rc_run) do { \
>>>>+ apr_status_t rc = rc_run; \
>>>>+ if (rc != APR_SUCCESS) { \
>>>>+ ap_log_error(APLOG_MARK, APLOG_ERR, rc, NULL, "failed: "); \
>>>>+ } \
>>>>+ } while (0)
>>
>>[...]
>>
>>>Thanks, Stas! That is a good check ...
>>
>>well, ideally it should abort the execution as well...
>>
>>Also may be APREQ_ASSERT is not the best name, in mp2 it's called
>>MP_FAILURE_CROAK()... may be APREQ_STATUS_CHECK? or something like that?
>
>
> Perhaps APREQ_STATUS_CHECK?
Sounds fine, the shorter the better. APREQ_STATCHK, probably hard to spel.
APREQ_CHECK? APREQ_IS_OK? APREQ_OK?
> I'm not sure it should abort, though, at least for
> apr_get_env(); from what I gather, if the environment
> variable for a given key doesn't exist, it just returns
> that key (and sets the status accordingly), and it's up to
> the caller to deal with that case?
Note that we don't abort when the key is not returned, but when the status is
not APR_SUCCESS, which means that something went wrong, and not when the
variable is not set. I was suggesting this as a macro that can be used
everywhere in apreq C source files.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [apreq-2] libapreq_cgi and getenv
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 19 Oct 2003, Stas Bekman wrote:
> Randy Kobes wrote:
>
> >>it doesn't hurt to check for failure. Probabaly write a macro to avoid noise
> >>(adopted from mp2):
> >>
> >>+ #define APREQ_ASSERT(rc_run) do { \
> >>+ apr_status_t rc = rc_run; \
> >>+ if (rc != APR_SUCCESS) { \
> >>+ ap_log_error(APLOG_MARK, APLOG_ERR, rc, NULL, "failed: "); \
> >>+ } \
> >>+ } while (0)
> [...]
> > Thanks, Stas! That is a good check ...
>
> well, ideally it should abort the execution as well...
>
> Also may be APREQ_ASSERT is not the best name, in mp2 it's called
> MP_FAILURE_CROAK()... may be APREQ_STATUS_CHECK? or something like that?
Perhaps APREQ_STATUS_CHECK?
I'm not sure it should abort, though, at least for
apr_get_env(); from what I gather, if the environment
variable for a given key doesn't exist, it just returns
that key (and sets the status accordingly), and it's up to
the caller to deal with that case?
--
best regards,
randy
Re: [apreq-2] libapreq_cgi and getenv
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
>>it doesn't hurt to check for failure. Probabaly write a macro to avoid noise
>>(adopted from mp2):
>>
>>+ #define APREQ_ASSERT(rc_run) do { \
>>+ apr_status_t rc = rc_run; \
>>+ if (rc != APR_SUCCESS) { \
>>+ ap_log_error(APLOG_MARK, APLOG_ERR, rc, NULL, "failed: "); \
>>+ } \
>>+ } while (0)
[...]
> Thanks, Stas! That is a good check ...
well, ideally it should abort the execution as well...
Also may be APREQ_ASSERT is not the best name, in mp2 it's called
MP_FAILURE_CROAK()... may be APREQ_STATUS_CHECK? or something like that?
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [apreq-2] libapreq_cgi and getenv
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 19 Oct 2003, Stas Bekman wrote:
> Randy Kobes wrote:
> > Hi,
> > I've been looking at libapreq_cgi, and am having some
> > problems getting cookie data from within a cgi program
> > (which I'll describe in a separate message). I don't know if
> > this is related, but would there be any objection (or harm?)
> > in using, within libapreq_cgi.c, the apr function
> > apr_env_get(), rather than getenv(), as in the following:
>
> +1, apr_env_get is better since it doesn't require running Perl ;) and we use
> it in mp2. It's probably faster as well...
>
> Though since you get a status anyway:
>
> > s = apr_env_get(&value, "QUERY_STRING", ctx->pool);
>
> it doesn't hurt to check for failure. Probabaly write a macro to avoid noise
> (adopted from mp2):
>
> + #define APREQ_ASSERT(rc_run) do { \
> + apr_status_t rc = rc_run; \
> + if (rc != APR_SUCCESS) { \
> + ap_log_error(APLOG_MARK, APLOG_ERR, rc, NULL, "failed: "); \
> + } \
> + } while (0)
> {
> - return getenv("QUERY_STRING");
> + dCTX;
> + char *value;
> + APREQ_ASSERT(apr_env_get(&value, "QUERY_STRING", ctx->pool));
> + return value;
> }
>
> may be a better error message...
Thanks, Stas! That is a good check ...
--
best regards,
randy