major bugfixes to array.c and strdict.c

[GoogleCodeExporter]

array.c and strdict.c are pretty messed up.

px_strdict_get() currently returns the strdict's internal key+value array
rather than returning the value itself. This would cause all sorts of
lossage, except for the fact that px_array_add() is broken and increments
self->length *before* adding the new member rather than after, which means
that self->data[0] will always be NULL, which means that px_array_find()
will always return -1 and so px_strdict_get() will always return NULL
(which, in turn means that proxy_factory_misc_get() always returns NULL. Um.)

px_array_del() is also buggy, in that it doesn't decrement self->length,
though since it uses px_array_find(), this doesn't matter much since you
can't delete things anyway.

Given that pxArray keeps track of its length, there's no need to
NULL-terminate the array as well, so I changed it all to not do that (which
as a side benefit means you can also store NULLs in pxArrays now if you want).

I also changed px_strdict_get() to just return NULL if you pass a NULL
strdict, to match px_strdict_set()'s behavior. (This is needed if you have
an empty or unparseable proxy.conf; without this change libproxy will crash
after reading it.)

There should really really be a regression test for these, but I haven't
written one yet.

Original issue reported on code.google.com by dan.wins...@gmail.com on 6 Feb 2009 at 5:56

Attachments:

• array-and-dict-fixes.diff

[GoogleCodeExporter]

merged with svn commit 304

Original comment by dominiqu...@gmail.com on 10 Feb 2009 at 9:36

• Changed state: Verified

[GoogleCodeExporter]

Dan,

since this patch has been applied to svn, the test program (bin/proxy) crashes 
here
on px_proxy_free. Can you reproduce this?

I reverted (on my local tree only) the part of the patch which touches array.c 
and
the segfault disappears.

Interestingly, the segfault also disappears when I remove the webkit plugin. You
might not have it and thus not trigger the segfault at yours.

Original comment by dominiqu...@gmail.com on 27 Feb 2009 at 10:01

• Changed state: Accepted

[GoogleCodeExporter]

Nope, doesn't crash for me.

webkit's on_proxy_factory_destantiate() does:

    ctxs_free(px_proxy_factory_misc_get(self, "webkit"));

before the patches, that would have been a no-op, so I think it's likely that 
the
strdict fixes are exposing a bug in the webkit plugin.

Skimming the code, it looks like if the webkit plugin tries to run a PAC script 
but
gets an error, then this would cause a crash later, because webkit_pacrunner() 
calls
ctxs_free() in that case, but doesn't call px_proxy_factory_misc_set(self, 
"webkit",
NULL), so the same context will get freed again later.

Original comment by dan.wins...@gmail.com on 4 Mar 2009 at 9:20

[GoogleCodeExporter]

Dan.. Thanks for the explanation.

I'll create a follow-Up ticket for the webkit plugin. Closing this ticket here 
again. 

Original comment by dominiqu...@gmail.com on 11 Mar 2009 at 3:53

• Changed state: Fixed