<<< Date Index >>>     <<< Thread Index >>>

Re: [PATCH 03 of 16] Since contex_sb might be uninitialized, it is



>> # HG changeset patch
>> # User Erik Hovland <erik@xxxxxxxxxxx>
>> # Date 1236890095 25200
>> # Branch HEAD
>> # Node ID b4cf86788470dceb7bc4f80edc7f9b7098839a43
>> # Parent  abb26dfac5656d678607e215f36a1c52db460a19
>> Since contex_sb might be uninitialized, it is
>> better to zero it and remove the condition
>> that does that.
>
> I think this patch is bad:
>
>> diff --git a/buffy.c b/buffy.c
>> --- a/buffy.c
>> +++ b/buffy.c
>> @@ -260,6 +260,9 @@
>>  struct stat contex_sb;
>>  time_t t;
>>
>> +  contex_sb.st_dev=0;
>> +  contex_sb.st_ino=0;
>> +
>> #ifdef USE_IMAP
>>  /* update postponed count as well, on force */
>>  if (force)
>> @@ -285,12 +288,6 @@
>> #ifdef USE_POP
>>  if (!Context || Context->magic != M_POP)
>> #endif
>> -  /* check device ID and serial number instead of comparing paths */
>> -  if (!Context || !Context->path || stat (Context->path, &contex_sb) !=
>> 0)
>> -  {
>> -    contex_sb.st_dev=0;
>> -    contex_sb.st_ino=0;
>> -  }
>>
>>  for (tmp = Incoming; tmp; tmp = tmp->next)
>>  {
>
> With POP or IMAP compiled in, you'll have two "if" statements checking
> for POP and/or IMAP. Also, you remove the stat() call completely and
> thus break the mailbox polling to exclude the current folder (for which
> contex_sb is only used).
>
> And since contex_sb is only used for local folders and stat() is only
> called for local folders I think it cannot actually be uninitialized.
>
> But I may be wrong.

I don't disagree w/ you. But the result of the if statement is useless if
the value is initialized before hand. The real solution would probably
be to set st_dev and st_ino to -1 and then check for that later, realizing
another error path. But I don't know if it is worth adding that extra
code. I will attempt a better fix for this one . Thanks for the feedback.

E

-- 
Erik Hovland
erik@xxxxxxxxxxx
http://hovland.org/