Patch for passing NULL as value argument to apr_env_get

[César Leonardo Blum Silveira <ce...@gmail.com>]

Hello,

Some time ago I wrote a piece of code which only needed to check
whether and environment variable was defined. Since I did not care
about the value the variable contained, I was a little annoyed by the
fact that I had to declare a variable only to pass it as the value
argument to apr_env_get. So I decided to modify the source in order to
only set the value variable when it is passed, i.e. is not NULL.

I see this is somewhat contrary to what the guidelines say, "One
notable standard that has been adopted within APR functions (and is
partly due to APR's httpd lineage) is that the input values do not
need to be checked for correctness.". However, I still think my
modification might be useful to other people.

Attached is the patch file. This is the first patch I ever made for
APR, so I don't know if it is precisely correct. Please review the
filenames in it.

Regards,

-- 
César L. B. Silveira

Re: Patch for passing NULL as value argument to apr_env_get

[Bojan Smojver <bo...@rexursive.com>]

On Wed, 2008-04-30 at 22:51 +0200, Branko Čibej wrote:
> He's proposing to make NULL a 
> well-defined parameter for this function, not an incorrect one.

Yeah, that's another way to see it. I was referring to the current
behaviour, where NULL is considered an incorrect value (together with a
value pointing nowhere), therefore causing a segfault. So, handling NULL
specially is avoiding the segfault.

-- 
Bojan

Re: Patch for passing NULL as value argument to apr_env_get

[Branko Čibej <br...@xbc.nu>]

Bojan Smojver wrote:
> On Tue, 2008-04-29 at 22:41 -0300, César Leonardo Blum Silveira wrote:
>   
>> I'm really sorry if this is annoying, but no one gave any feedback on
>> this. Hasn't anyone liked the idea? Or maybe I should log this on
>> bugzilla?
>>     
>
> I think this in unlikely to be included, as it changes the usual APR
> behaviour (which is something you noted yourself). Usually, we just let
> things segfault if values passed into the function are not correct.
>
> BTW, you can solve your problem by wrapping APR functions into your own,
> where you do perform the checks and/or define needed variables
> correctly.
>   

This answer isn't really very helpful. He's proposing to make NULL a 
well-defined parameter for this function, not an incorrect one.

On the other hand, I generally object to overloading semantics depending 
on parameter values. In this particular case the proper way would be to 
add a new public function that does just the existence check.

-- Brane

Re: Patch for passing NULL as value argument to apr_env_get

[Bojan Smojver <bo...@rexursive.com>]

On Tue, 2008-04-29 at 22:41 -0300, César Leonardo Blum Silveira wrote:
> I'm really sorry if this is annoying, but no one gave any feedback on
> this. Hasn't anyone liked the idea? Or maybe I should log this on
> bugzilla?

I think this in unlikely to be included, as it changes the usual APR
behaviour (which is something you noted yourself). Usually, we just let
things segfault if values passed into the function are not correct.

BTW, you can solve your problem by wrapping APR functions into your own,
where you do perform the checks and/or define needed variables
correctly.

-- 
Bojan

Re: Patch for passing NULL as value argument to apr_env_get

[César Leonardo Blum Silveira <ce...@gmail.com>]

I'm really sorry if this is annoying, but no one gave any feedback on
this. Hasn't anyone liked the idea? Or maybe I should log this on
bugzilla?

Thank you,

-- 
César L. B. Silveira


On Fri, Apr 25, 2008 at 1:24 PM, César Leonardo Blum Silveira
<ce...@gmail.com> wrote:
> Hello,
>
>  Some time ago I wrote a piece of code which only needed to check
>  whether and environment variable was defined. Since I did not care
>  about the value the variable contained, I was a little annoyed by the
>  fact that I had to declare a variable only to pass it as the value
>  argument to apr_env_get. So I decided to modify the source in order to
>  only set the value variable when it is passed, i.e. is not NULL.
>
>  I see this is somewhat contrary to what the guidelines say, "One
>  notable standard that has been adopted within APR functions (and is
>  partly due to APR's httpd lineage) is that the input values do not
>  need to be checked for correctness.". However, I still think my
>  modification might be useful to other people.
>
>  Attached is the patch file. This is the first patch I ever made for
>  APR, so I don't know if it is precisely correct. Please review the
>  filenames in it.
>
>  Regards,
>
>  --
>  César L. B. Silveira
>