take care 64-bits (!= 32-bits) time_t

[brong]

From: OBATA Akio Bugzilla-Id: 3376 Version: 2.4.6 Owner: Ken Murchison

[brong]

From: OBATA Akio

On NetBSD-current (will be NetBSD-6.0), time_t is 64-bits. But I feel time_t is expected as 32-bits in Cyrus-IMAP sources. I'm not sure whether it should be exactly 32-bits (for protocol reason? from IMAP spec, and doc/internal/replication_protocol.html) or native time_t value for any case.

For example, in imap/mailbox.c:1892, time_t is treated as "%lu" for snprintf. In this case, it will crash because a half of record->internaldate will be used as the next "%s". Then, how to fix? record->* should be unsigned long? or introduce platform depend TIME_T_FMT?

[brong]

From: OBATA Akio

Not just 64-bits and 32-bits time_t issue, time_t != long int platforms (32-bits arch, but time_t is 64-bits).

[brong]

Attachment-Id: 1536 From: Antoine Jacoutot Type: text/plain File: patch-imap_mailbox_c.txt

cast time_t to long long

[brong]

From: Antoine Jacoutot

On OpenBSD we are using this patch to prevent a crash when delivering with cyrusv2 and sendmail. IIRC casting time_t to long long is the most portable way to represent time values.

[brong]

From: OBATA Akio

(In reply to comment #3) > On OpenBSD we are using this patch to prevent a crash when delivering with > cyrusv2 and sendmail. IIRC casting time_t to long long is the most portable way > to represent time values.

I'm using cyrusimap in pkgsrc, i.e. need multiple platform support. Moreover, current souce codes use "HAVE_LONG_LONG_INT" switch in various location. So unconditional cast to "unsigned long long" is probably not acceptable.

Anyway, you should also patch to log_record() in imap/sync_client.c, but you can find "unsinged long" is used by sieve_upload() as a argment. It may also be changed to "time_t", because "last_updated" is passed to dlist_date(), and its argment is defined as "time_t". Other such mismatch also may be found.

[brong]

From: OBATA Akio

Here are current our patches, using modseq_t and MODSEQ_FMT, same as imap/dlist.c:

--- imap/sync_client.c.orig 2012-12-01 19:57:54.000000000 +0000 +++ imap/sync_client.c @@ -573,7 +573,7 @@ static int folder_unannotation(const cha / ====================================================================== /

static int sieve_upload(const char userid, const char filename,

• unsigned long last_update)
• time_t last_update) { const char cmd = "SIEVE"; struct dlist kl; @@ -913,9 +913,9 @@ static void log_record(const char name, struct index_record record) { syslog(LOG_NOTICE, "SYNCNOTICE: %s uid:%u modseq:" MODSEQ_FMT " "
• "last_updated:%lu internaldate:%lu flags:(%s)",
• "last_updated:" MODSEQ_FMT " internaldate:" MODSEQ_FMT " flags:(%s)", name, record->uid, record->modseq,
• record->last_updated, record->internaldate,
• (modseq_t)record->last_updated, (modseq_t)record->internaldate, make_flags(mailbox, record)); }

--- imap/mailbox.c.orig 2012-12-01 19:57:54.000000000 +0000 +++ imap/mailbox.c @@ -1960,10 +1960,10 @@ bit32 make_sync_crc(struct mailbox *mail flagcrc ^= crc32_cstring(buf); }

• snprintf(buf, 4096, "%u " MODSEQ_FMT " %lu (%u) %lu %s",
• record->uid, record->modseq, record->last_updated,
• snprintf(buf, 4096, "%u " MODSEQ_FMT " " MODSEQ_FMT " (%u) " MODSEQ_FMT " %s",
• record->uid, record->modseq, (modseq_t)record->last_updated, flagcrc,
• record->internaldate,

• (modseq_t)record->internaldate, message_guid_encode(&record->guid));

  return crc32_cstring(buf);

[brong]

From: Antoine Jacoutot

> So unconditional cast to "unsigned long long" is probably not acceptable.

Well as far as I know casting to long long it's the only way that it will work on most UNIX OSes. Granted that may require more work in the cyrus case.

> Anyway, you should also patch to log_record() in imap/sync_client.c, > but you can find "unsinged long" is used by sieve_upload() as a argment. > It may also be changed to "time_t", because "last_updated" is passed to > dlist_date(), and its argment is defined as "time_t". > Other such mismatch also may be found.

Yeah unfortunately there are lots of places where the use of time_t is wrong... I have no time to dig but I will have a look at your patches for the OpenBSD package at least. Since this bug was opened almost 4 years ago, I doubt it is of interested to the devs.

Thanks.