[asterisk-dev] [svn-commits] jpeeler: branch 1.6.2 r303677 - in /branches/1.6.2: ./ apps/app_voicemail.c
Jeff Peeler
jpeeler at digium.com
Tue Feb 8 10:21:11 CST 2011
Well I guess you are being paranoid for the int overflow (unless MAXMSGLIMIT is modified), but that's OK. Thanks for pointing it out!
Fixed in r306864.
----- Original Message -----
> 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
> >>
> >
> >
> >
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
> http://lists.digium.com/mailman/listinfo/asterisk-dev
--
Jeff Peeler
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at www.digium.com & www.asterisk.org
More information about the asterisk-dev
mailing list