On 04/05/2008 05:26 PM, Graham Leggett wrote:
Ruediger Pluem wrote:

+AP_DECLARE(int) ap_unescape_all(char *url);
+
+/**

Doesn't this require a minor bump?

I think it does do, yes. Fixed.

+AP_DECLARE(char *) ap_escape_path_segment_b(char *c, const char *s);

This is not a very descriptive name. Shouldn't it be better ap_escape_path_segment_buffer?

It was originally buffer, then I changed it because it was too long. You're right though, will change.

Just saw it. I guess you missed to fix the name in the modules that call it.


+AP_DECLARE(void) ap_session_get(request_rec * r, session_rec * z, const char *key, const char **value)
+{
+    if (!z) {
+        ap_session_load(r, &z);

Not checking the return value can lead to segfaults as z can be invalid.

The real check is to ensure that z is set, the return value can be successful but z can still be NULL (which will happen if no session is
configured), so checking the return value won't help.

Ok, but then we should ensure that z will be at least NULL for proper checking.
AFAICT we do not set *z to NULL in ap_session_load in the case of an error.
So it z may point to an arbitrary location in the memory.

Regards

Rüdiger


Reply via email to