[sf#86] Avoid putenv() in libical

[Milan Crha]

        Hello,
you've filled:
   libical messes with timezone, affecting other threads [sf#86] #86
   https://github.com/libical/libical/issues/86
but I do not have an account there, and I'm not going to have one in 
the near future, thus I'm writing rather here (I'm also not subscribed 
on the list, thus please keep me in CC when/if replying).

There seems to be an increasing issue with this, in connection with 
evolution-data-server, which uses libical and is highly multi-
threaded. One recent valgrind log shows this:

  Invalid read of size 8
     at 0xAEACBA4: getenv (getenv.c:76)
     by 0xAEA3141: guess_category_value (dcigettext.c:1356)
     by 0xAEA3141: __dcigettext (dcigettext.c:561)
     by 0x31986744: ??? (in /usr/lib64/gio/modules/libgiognutls.so)
     by 0xC5D8771: g_input_stream_read (ginputstream.c:195)
     by 0xC5D8771: g_input_stream_read (ginputstream.c:195)
     by 0xC2F804E: soup_body_input_stream_read_raw (soup-body-input-stream.c:131)
     by 0xC2F804E: soup_body_input_stream_read_chunked (soup-body-input-stream.c:183)
     by 0xC2F804E: read_internal (soup-body-input-stream.c:250)
     by 0xC5D8771: g_input_stream_read (ginputstream.c:195)
     by 0xC3113E0: io_read (soup-message-io.c:646)
     by 0xC311828: io_run_until (soup-message-io.c:864)
     by 0xC312254: io_run (soup-message-io.c:936)
     by 0xC30F044: soup_message_send_request (soup-message-client-io.c:161)
     by 0xC320E5B: soup_session_process_queue_item (soup-session.c:1964)
   Address 0x33700a68 is 296 bytes inside a block of size 408 free'd
     at 0x4C2BB1C: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
     by 0xAEACDAB: __add_to_environ (setenv.c:141)
     by 0xAEACC50: putenv (putenv.c:78)
     by 0x4C31027: putenv (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
     by 0xB47DE1E: set_tz (icaltime.c:342)
     by 0xB47EB09: icaltime_as_timet_with_zone (icaltime.c:435)
     by 0x50A55B7: time_from_isodate (e-cal-time-util.c:656)
     by 0x4E4EFC1: e_cal_backend_sexp_func_make_time (e-cal-backend-sexp.c:1392)
     by 0x557C410: e_sexp_term_eval (e-sexp.c:783)
     by 0x557C3E3: e_sexp_term_eval (e-sexp.c:779)
     by 0x557CC6C: term_eval_and (e-sexp.c:287)
     by 0x557C452: e_sexp_term_eval (e-sexp.c:771)

(taken from https://bugzilla.gnome.org/show_bug.cgi?id=701138#c16 )

I understand the above valgrind claim as a thread interleave of 
libical's putenv() call and libgiognutls' (indirect) getenv() call. 
There are few libraries involved in this, as you can see. It also 
shows that any locking on the libical side is useless when the final 
application uses more than just libical. (That's only to mention, not a
complain.)

I tried to address this with as less pain as possible and I feel I 
found a solution. Let me show you what I've done:

a) I applied the attached test.patch to libical 1.0. It extends 
libical's make_time() function whether it should be strict (take care 
of integer overflow) or not. The 64bit systems (like Linux) define 
time_t as 8 byte sized, thus there the limits do not apply, thus I 
added the check to it too. Then I used the modified make_time() in 
icaltime_as_timet_with_zone(), but only to compare whether the time_t 
returned from this function matches the one from mktime(), that's the 
assert() at the end.

b) to verify the test.patch I created a simple ical.c test program 
(attached). It just calls icaltime_as_timet_with_zone(), which does 
the actual test whether the values match, for various zones and times 
during the year. If they do not match, then the application aborts. It 
doesn't abort on my 64bit Linux system during the whole run.

c) the above led me to the final proposed.patch (attached), which 
removed the putenv() call completely. Actually, there are two places 
where it's left, in:
   libical-1.0/src/test/regression.c:3791
   libical-1.0/src/test/timezones.c:61
but those are just tests, no "production" code. The "make test" 
finished properly with this change too.

Please, consider using this proposed.patch in the next libical release 
(when is the 1.1 planned, by the way?).

        Bye,
        Milan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.patch
Type: text/x-patch
Size: 2297 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20141126/96328bfd/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ical.c
Type: text/x-csrc
Size: 1754 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20141126/96328bfd/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: proposed.patch
Type: text/x-patch
Size: 5544 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20141126/96328bfd/attachment-0002.bin>