httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c apr_tables.c
Date Wed, 24 Nov 1999 13:45:32 GMT
On Wed, 24 Nov 1999, Greg Marr wrote:
>...
> >By "talking Apache here", I meant "Apache-using-APR". In that 
> >condition,
> >everything ever written for Apache assumes that a memory allocation 
> >will
> >SUCCEED. So, in light of Apache... yes, we should be killing the 
> >process
> >if we can't get the memory.
> 
> If that's what Apache wants to do, then Apache should kill the 
> process when a malloc fails, not APR.

With the hook approach, this is reasonable to do. I do not see any other
way that would be convenient.

>...
> >And I say no. The current model of "alloc and you'll get it" means 
> >that Apache can be very fast. It doesn't have to worry about not 
> >getting memory.
> 
> Yes it does, just in a different place.  Something has to decide that 
> Apache should log the error and die.  You're saying that APR is that 
> place.  Ryan is saying that Apache is that place.  In either case, 
> someone has to do it.  In order for APR to be usable for anything 
> outside of Apache, that place needs to be Apache.

If you do it at the bottom, then you check once and die. Nobody else has
to ever check. I'm against the model where you have to check the status
code all the way back up the stack.

> >If you *do* have to worry about it, then you start putting checks on 
> >every darn function call.
> 
> This is a bad thing?

Certainly.

I don't have to do it now. It is a significant, incremental effort to add
these checks throughout my code. That is a bad thing.

No, the checks in-and-of themselves are not "bad", but I believe the
current programming model in Apache is much better. A developer doesn't
have to worry about a malloc failure, and (unrelated to this particular
thread) they don't have to worry about freeing resources (and various ways
to cleanup on abnormal function exit).

> >You ever see what that looks like? Go look at some COM code in 
> >Windows. You have one line of work, three lines of error 
> >handling.  It is absolutely horrible.
> 
> I program in Windows for a living.

I used to.

> We use a library that used to like to abort when something went.  It 
> was a royal pain.  We finally convinced them that aborting was a bad 
> thing, since our customers see it as an error on our part.

For a library like that... yes, it is a bad thing. Never said that
libraries should do this. I'm saying *Apache* should abort on memory alloc
failure. If the lowest point in the calltree happens to be APR, then it
should be doing that for us.

> >Further, the time to check a result, when it is typically successful 
> >is just wasted time.
> 
> You are kidding, right?  Please tell me you're kidding.

No, I'm not. Yah... individual checks are cheap. Keep doing it
*everywhere* and they add up. Realize that it is more than just a check:
typically you have storage into a value (for propagating that error out).
If these checks occur within a loop, then again: they add up.

Sure, a percent or two. But that matters to some people :-)

I'm more against the checks for aesthetic purposes, than the performance
problem. But I had to mention it :-)

> >Lastly, people will just start getting lazy and not putting in 
> >checks.  Then you end up with a case where a NULL pointer gets hit 
> >at some arbitrary point in the code, a long ways away from the 
> >(failed) allocation. Tracking that back is a bitch.
> 
> That is why you need to check return codes.

"need" is a great ideal. But that isn't how things truly work. People
*will* skip checks.

> >Changing return codes is one thing. Changing the basic programming 
> >model and semantics of Apache development is something else entirely.
> 
> That is why it needs to be done as few times as possible.  Get all of 
> the changes in at once.

If the decision to alter the basic model is accepted in the first place --
yes, do it now rather than later.

> >I could be mistaken on Ryan's intent (and I asked him that in my 
> >last email), but if we are really talking about needing to check a 
> >status code from ap_pstrup(), then I'll be quite upset.
> 
> You mean you don't check return codes now?

Of course not! ap_pstrdup() does NOT have a return code! That's my point
here. Take the following line of code:

  ap_log_rerror(...,
                ap_psprintf(r->pool, "error at URL \"%s\".",
                            ap_escape_html(r->pool, r->uri)));

Unwind that sucker and put in checks. Ack! No thank you. What is currently
a very clean piece of code would become wordy and obnoxious.

>...
> >Sure, APR is a library, but it is still an integral part of Apache 
> >and one of its goals is to be well-tuned for Apache's needs.
> 
> Well tuned, yes.  Pigeon-holed into, no.

Programming model change, no.

> >And yes, I think a malloc should bring things down. It is a better 
> >model than boatloads of memory-status checks.
> 
> I don't agree.  It may not be as critical in Apache's case, but if my 
> company's main product exited when a malloc failed, then customers 
> would lose data.

Well, come on. Of course. I'm not suggesting that this style is used
outside of Apache.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/


Mime
View raw message