Raphael,

I went over your patches from
http://patch-tracker.debian.org/package/php5/5.3.1-2 and did quick
reviews (didn't apply / test them or anything ...)

Here some comments:

004-ldap_fix.patch:
  Do you have a test for this? when does it happen that ldap_value is
  NULL happen? - If that's an issue we should add it to our tree...

013-force_getaddrinfo.patch:
  Why doesn't the check work on Debian out of the box? On my (not
  Debian) Linux boxes it works. anything needed for people compiling PHP
  on Debian themselves?

019-z_off_t_as_long.patch
  Certainly nothing for our tree

034-apache2_umask_fix.patch
036-fd_setsize_fix.patch
  Any reproduce case where this is an issue?

043-recode_size_t.patch
  Why is that needed? The name of the patch suggests that str_len is
  used in a place where an size_t is expected but then the location
  of this check is wrong. Additionally in case this check is tirggered
  the function will return without error message.

045-exif_nesting_level.patch
  How did you decide on the limit of 250?

047-zts_with_dl.patch
  I don't support this. dl() can be used by CLI users for loading Gtk or
  such on demand but avoiding to load (and inititalize) all the Gtk
  stuff every time they use PHP and avoid messing with config files...

052-phpinfo_no_configure.patch
  Neither do I support this. Users might benefit when building own
  modules or might be interested in the configure line for other
  reasons ...

053-extension_api.patch
  When adding random options to command line tools please mark them as
  Debian specific. Additionally the man page should be updated.

100-recode_is_shared.patch
  The conflict between MySQL and recode should only happen with an old
  libmysql (3.23?) not sure about imap ... but in your case the patch
  might make sense, while I won't directly apply it to our tree as
  usually people will build extensions to load them together...

101-sqlite_is_shared.patch
  Not sure if we do any modifications to SQLite but in general we prefer
  to use the bundled lib so users get the same behavior on all
  platforms.

108-64_bit_datetime.patch
116-posixness_fix.patch
  Why is that needed on our system? - I never saw an issue about these
  and if they are missing there should be build issues.

use_embedded_timezonedb.patch
  Like with the SQLite note above we prefer to have the same behavior
  over all platforms by using a common time database.

force_libmysqlclient_r.patch
  Why are you using the reentrant version of this library which is
  slower than the "regular" one? Did you consider mysqlnd?

009_ob-memory-leaks.patch
   Any test case? (While this looks good on first sight)

exif_read_data-segfault.patch
   Any sample case for this? Any bug report?

sybase-alias.patch
   I have no idea about MS SQL and Sybase but "randomly" aliassing looks
   bad to me.

strcmp_null-OnUpdateErrorLog.patch
   You should at least ad an "echo 'done';" or such to the expect
   section to make sure it really works ...

I skipped the patches related to build system and such and maybe skipped
one or two by accident ....

johannes


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to