[php-maint] Bug#535770: Bug#535770: PHP system timezone patch for 5.3?

[sean finney]

hi joe,

some quick notes/observations...

in your comments you say:

	/* Parse an ISO-6709 date as used in zone.tab. Returns end of the

s/date/coordinate/ ? :)

but more seriously:

* in create_zone_index() it looks like
  find_zone_info(system_zone_info...) is called but system_zone_info
  isn't initialized until after create_zone_index has returned.

* i'm not sure but it seems like there might be a few corresponding free()'s
  missing from the mallocs/strdups.

* it looks like load_zone_table() is called unconditionally, even if
  it's not needed (i.e. a scall to localtime() will trigger it)

* is parsing the comments really necessary?  i don't see any php functions
  that use them.

also, before i came to the conclusion that some kind of intermediate
hashtable was needed to do the mapping (at which point i gave up and
figured i'd wait to hear back from you :), i had done a slightly 
simpler implementation of the parsing using a clean mmap'd buffer
and sscanf with "%ms" type formats which avoids a lot of the hard-coded
lengths etc[1].  if you're interested i can hack that code into the
load_zone_info (and parse_iso6709) to make a leaner/cleaner/more efficient
implementation.

also, FYI my posts to the fedora list are being automatically rejected
so if i say anything important that should be shared someone should
forward it along :)


	sean

[1] 'm' being the sooner-or-later POSIX.1 adoption of the non-standard 'a'
    GNU libc extension to automatically allocate the parsed strings, which 
    in turn conflicted with the C99 'a' (float) format specifier, hence
    the new specifier.  it requires glibc >= 2.7 btw.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-php-maint/attachments/20090715/49e6ab40/attachment-0003.pgp>