[asterisk-dev] [svn-commits] jpeeler: branch 1.6.2 r303677 - in /branches/1.6.2: ./ apps/app_voicemail.c

Steve Davies davies147 at gmail.com
Tue Feb 8 06:04:26 CST 2011


Oops. Tried to send this to asterisk-commits by accident.

On 8 February 2011 12:04,  <svn-commits-owner at lists.digium.com> wrote:
> ---------- Forwarded message ----------
> From: Steve Davies <davies147 at gmail.com>
> To: SVN commits to the Digium repositories <svn-commits at lists.digium.com>, Jeff Peeler <jpeeler at digium.com>
> Date: Tue, 8 Feb 2011 12:02:56 +0000
> Subject: Re: [svn-commits] jpeeler: branch 1.6.2 r303677 - in /branches/1.6.2: ./ apps/app_voicemail.c
> Hi,
>
> Regarding the patch below, I have some comments/questions.
>
> 1) The sscanf of
>    "%3s" -> extension
> in the sscanf, will be null terminated according to the sscanf
> man-page, so surely extension[3] should be declared as extension[4]?
>
> 2) Extension names in the voicemail directory can be longer than 3
> characters. In order to future-proof this code, should we allow "%4s"
> or longer as the extension pattern, to cater for .g729, .alaw and
> other already longer extensions? Of course we only appear to be
> interested in ".txt" in this loop, so this may be pointless.
>
> 3) The %30d can potentially overflow an 'int' type. Is it safer to use
> %9d, which will always fit?
>
> Hope I am not just being daft/paranoid!
>
> Regards,
> Steve
>
>
> On 25 January 2011 16:59, SVN commits to the Digium repositories
> <svn-commits at lists.digium.com> wrote:
>> Author: jpeeler
>> Date: Tue Jan 25 10:59:28 2011
>> New Revision: 303677
>>
>> URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=303677
>> Log:
>> Merged revisions 303676 via svnmerge from
>> https://origsvn.digium.com/svn/asterisk/branches/1.4
>>
>> ........
>>  r303676 | jpeeler | 2011-01-25 10:58:29 -0600 (Tue, 25 Jan 2011) | 20 lines
>>
>>  Fix voicemail sequencing for file based storage.
>>
>>  A previous change was made to account for when the number of voicemail messages
>>  exceeds the max limit to be handled properly, but it caused gaps in the messages
>>  to not be properly handled. This has now been resolved.
>>
>>  In later non 1.4 branches, it appears that resequencing wasn't even occurring
>>  due from what appears and accidental code removal.
>>
>>  (closes issue #18498)
>>  Reported by: JJCinAZ
>>  Patches:
>>        bug18498v2.patch uploaded by jpeeler (license 325)
>>
>>  (closes issue #18486)
>>  Reported by: bluefox
>>  Patches:
>>        bug18486.patch uploaded by jpeeler (license 325)
>> ........
>>
>> Modified:
>>    branches/1.6.2/   (props changed)
>>    branches/1.6.2/apps/app_voicemail.c
>>
>> Propchange: branches/1.6.2/
>> ------------------------------------------------------------------------------
>> Binary property 'branch-1.4-merged' - no diff available.
>>
>> Modified: branches/1.6.2/apps/app_voicemail.c
>> URL: http://svnview.digium.com/svn/asterisk/branches/1.6.2/apps/app_voicemail.c?view=diff&rev=303677&r1=303676&r2=303677
>> ==============================================================================
>> --- branches/1.6.2/apps/app_voicemail.c (original)
>> +++ branches/1.6.2/apps/app_voicemail.c Tue Jan 25 10:59:28 2011
>> @@ -3762,6 +3762,8 @@
>>        DIR *msgdir;
>>        struct dirent *msgdirent;
>>        int msgdirint;
>> +       char extension[3];
>> +       int stopcount = 0;
>>
>>        /* Reading the entire directory into a file map scales better than
>>         * doing a stat repeatedly on a predicted sequence.  I suspect this
>> @@ -3772,14 +3774,20 @@
>>        }
>>
>>        while ((msgdirent = readdir(msgdir))) {
>> -               if (sscanf(msgdirent->d_name, "msg%30d", &msgdirint) == 1 && msgdirint < MAXMSGLIMIT)
>> +               if (sscanf(msgdirent->d_name, "msg%30d.%3s", &msgdirint, extension) == 2 && msgdirint < MAXMSGLIMIT && !strcmp(extension, "txt")) {
>>                        map[msgdirint] = 1;
>> +                       stopcount++;
>> +                       ast_debug(4, "%s map[%d] = %d, count = %d\n", dir, msgdirint, map[msgdirint], stopcount);
>> +               }
>>        }
>>        closedir(msgdir);
>>
>>        for (x = 0; x < vmu->maxmsg; x++) {
>> -               if (map[x] == 0)
>> +               if (map[x] == 1) {
>> +                       stopcount--;
>> +               } else if (map[x] == 0 && !stopcount) {
>>                        break;
>> +               }
>>        }
>>
>>        return x - 1;
>> @@ -5749,6 +5757,36 @@
>>        ast_free(tmp);
>>        return res;
>>  }
>> +
>> +#ifndef IMAP_STORAGE
>> +static int resequence_mailbox(struct ast_vm_user *vmu, char *dir, int stopcount)
>> +{
>> +    /* we know the actual number of messages, so stop process when number is hit */
>> +
>> +    int x, dest;
>> +    char sfn[PATH_MAX];
>> +    char dfn[PATH_MAX];
>> +
>> +    if (vm_lock_path(dir))
>> +        return ERROR_LOCK_PATH;
>> +
>> +    for (x = 0, dest = 0; dest != stopcount && x < vmu->maxmsg + 10; x++) {
>> +        make_file(sfn, sizeof(sfn), dir, x);
>> +        if (EXISTS(dir, x, sfn, NULL)) {
>> +
>> +            if (x != dest) {
>> +                make_file(dfn, sizeof(dfn), dir, dest);
>> +                RENAME(dir, x, vmu->mailbox, vmu->context, dir, dest, sfn, dfn);
>> +            }
>> +
>> +            dest++;
>> +        }
>> +    }
>> +    ast_unlock_path(dir);
>> +
>> +    return dest;
>> +}
>> +#endif
>>
>>  static int say_and_wait(struct ast_channel *chan, int num, const char *language)
>>  {
>> @@ -7440,8 +7478,12 @@
>>
>>        if (last_msg < -1) {
>>                return last_msg;
>> -       } else if (vms->lastmsg != last_msg) {
>> -               ast_log(LOG_NOTICE, "Mailbox: %s, expected %d but found %d message(s) in box with max threshold of %d.\n", vms->curdir, last_msg + 1, vms->lastmsg + 1, vmu->maxmsg);
>> +       }
>> +#ifndef ODBC_STORAGE
>> +       else if (vms->lastmsg != last_msg) {
>> +               ast_log(LOG_NOTICE, "Resequencing mailbox: %s, expected %d but found %d message(s) in box with max threshold of %d.\n", vms->curdir, last_msg + 1, vms->lastmsg + 1, vmu->maxmsg);
>> +        resequence_mailbox(vmu, vms->curdir, count_msg);
>> +#endif
>>        }
>>
>>        return 0;
>>
>>
>> --
>> _____________________________________________________________________
>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>
>> svn-commits mailing list
>> To UNSUBSCRIBE or update options visit:
>>   http://lists.digium.com/mailman/listinfo/svn-commits
>>
>
>
>



More information about the asterisk-dev mailing list