Hello Nuno,
On 1/13/07, Nuno Lopes <[EMAIL PROTECTED]> wrote:
> On 1/13/07, Nuno Lopes <[EMAIL PROTECTED]> wrote:
>> nlopess Sat Jan 13 11:16:15 2007 UTC
>>
>> Modified files: (Branch: PHP_5_2)
>> /php-src/ext/gd/libgd gd.c
>> Log:
>> fix valgrind error in test bug24594.phpt
>> while at it, remove some dead code and change the pts vector to char to
>> save (much) memory
>> # Pierre: one more to merge ;)
>
> Thanks for the catch. I like to merge (and keep the fix) if I know
> what you are fixing. Can you send me the details please?
As the commit log says, I fixed a few valgrind warnings in the bug24594.phpt
test (it was reading one past the end of the array). The log is still
available at
http://gcov.php.net/viewer.php?version=PHP_5_2&func=valgrind&file=ext%2Fgd%2Ftests%2Fbug24594.phpt
the fix is this part:
- for (; x<=wx2 && (!pts[y][x] && gdImageGetPixel(im,x, y)==oc) ; x++) {
+ for (; x<wx2 && (!pts[y][x] && gdImageGetPixel(im,x, y)==oc) ; x++) {
What is the OS/compiler/cpu on this box? I remember that there was
warnings/errors there but nobody else was able to reproduce. Can you
reproduce this warning on your local box?
> About removing the dead code, again I do not like to do it during the
> RC phase. Please do it __after__ 5.2.1 (take #2) if there is a very
> good reason to do it, but I like to keep it (as I voluntary kept it).
It is really dead code and it was even marked as such. It is impossible
(excluding compiler/cpu/memory/.. bugs) to reach the code I removed.
If you feel scary with such trivial changes that means the extension hasn't
enough tests :P (ok, kidding. that part of the code is covered)
I did not say that this code is used or reached. I said that I liked
to keep it. I do feel good with this function (I will not rewrite the
old one and put my result in a stable branche if I was not), that's
not the question. I only liked to keep it or at the very least to put
it in a comment. It helps to quickly get the whole picture when you
read the function after a long time :)
>> OMG, this line was really bogus (and allocating huge amounts of memory
>> unnecessarily)
>
> It was more wrong after your change (int -> char) but as I said I did
> not see good reasons to change that now. Or is it critical and if yes
> how?
it is critical to my point of view :) Before my patch it would take 4 times
more memory than after the patch. With larger images, it is critical :P
The code can still be further optimized when using square images, but I
didn't deliberately make such change in an RC phase and without your
approval.
I like to first see a patch for this optimization. There is many
special cases or things to worry about here and BC is important. I'm
not saying that an optimization is not possible or welcome, only that
I like to see it prior to commit.
Cheers,
--Pierre
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
