On Thu, Mar 30, 2000 at 02:25:45PM +0100, Edmund GRIMLEY EVANS wrote:
> Vincent Lefevre <[EMAIL PROTECTED]>:
>
> > > - time_t t;
> > > + time_t t = 0; /* to avoid compiler warning */
> > >
> > > - /*
> > > - * gcc thinks it has to warn about uninitialized use
> > > - * of t. This is wrong.
> > > - */
> > > -
> >
> > I disagree. You shouldn't prevent warnings like this. If gcc thinks
> > that the variable is uninitialized, it should be fixed, and you
> > shouldn't add unnecessary code, that makes it bigger...
> You're right. My patch made the code bigger. By exactly one xor
> instruction on i386! But here's an alternative patch that makes the
> code smaller (but it is less obvious that this patch is safe):
Hmm... I thought the whole problem was just because the flow was
overly complex and it confused gcc (and those who might be reading the
source). I did this patch to clean it up, although I suspect the whole
thing could use a touch of refactoring based on how many branches are
in this function. This patch avoids putting the t variable on the
stack unless it's needed, and pulls it out of scope ASAP. It does
introduce one extra variable, but I suspect this incremental
improvement in code hygiene is more of a good thing than a bad one.
--Chris
--- imap.c.orig Thu Mar 30 14:42:54 2000
+++ imap.c Thu Mar 30 14:55:30 2000
@@ -1153,9 +1153,8 @@
int imap_check_mailbox (CONTEXT *ctx, int *index_hint)
{
char buf[LONG_STRING];
- static time_t checktime=0;
- time_t t;
-
+ short timedout = FALSE;
+
/*
* gcc thinks it has to warn about uninitialized use
* of t. This is wrong.
@@ -1163,15 +1162,20 @@
if (ImapCheckTimeout)
{
- t = time(NULL);
+ static time_t checktime=0;
+ time_t t = time(NULL);
+
t -= checktime;
+ if (t >= ImapCheckTimeout)
+ {
+ checktime += t;
+ timedout = TRUE;
+ }
}
- if ((ImapCheckTimeout && t >= ImapCheckTimeout)
+ if ((timedout)
|| ((CTX_DATA->reopen & IMAP_REOPEN_ALLOW) && (CTX_DATA->reopen &
~IMAP_REOPEN_ALLOW)))
{
- if (ImapCheckTimeout) checktime += t;
-
CTX_DATA->check_status = 0;
if (imap_exec (buf, sizeof (buf), CTX_DATA, "NOOP", 0) != 0)
