httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From André Malo ...@perlig.de>
Subject Re: Apache Code Inspection Report (fwd)
Date Tue, 01 Jul 2003 20:01:13 GMT
* Marc Slemko wrote:

> But yes, there are some issues they point out that should be fixed if they
> are still present.  Fixing them certainly won't significantly change the
> overall software quality though.

Well, let's have a look. My first analysis:

ID 1 seems to be invalid, the preconditions code path cannot happen.
(second and last precondition are in opposite of each other, because
current_provider == conf->providers in this case)

ID 2 ... hmm, am I blind or don't we have an abort_fn registered in our
pools? Why? That would solve a lot of problems, IMHO. If there's an
abort_fn registered, all IDs referencing to this one are invalid.

ID 3 is valid. We should bail out and return -1 if tag == NULL, I think.

ID 4: Not sure. What shall happen, if a bucket cannot be created?

ID 5: see ID 2

ID 6, 7, 8: see ID 4

ID 9: seems to be invalid. strchr(segstart, '\0') cannot return NULL on an
already valid C-string.

ID 10:  see ID 2

ID 11: see ID 4

ID 12: hmm, I don't seem to understand the critical loop... Could someone
explain it (why do we start with i=1?). An if(args) wrapper should solve it
anyway.

ID 13: we should leave it as is, because it shows us errors in the caller
functions. (would call it "by design").

ID 14: see ID 13

ID 15, 16, 17, 18: see ID 4

ID 19: ap_strchr_c(valid_string, '\0') cannot be NULL. see ID 9

ID 20: seems to be invalid?

ID 21: don't know, I'm not an APR guru ;-)

ID 22, 23: hmm. by design, IMHO, see ID 2

ID 24: see ID 4

ID 25: very valid!

ID 26: seems to be valid, needs an if (pTail) wrapper.

ID 27: see ID 22

ID 28: haha! Already fixed two days ago or so.

ID 29: should be fixed.

ID 30: errr, cannot happen, but we should bail out (assert?)

ID 31: correct, though I don't think we should double check n.

More opinions and reviews would be helpful, of course.

nd

Mime
View raw message