Re: [hylafax-users] ag: jobID/seqf

[Aidan Van Dyk <aidan@xxxxxxxx>]

* Bodo Meissner <bodo@xxxxxxxxx> [051123 09:34]:
 
> Hello Moritz and Erich,
> 
> the data type is not the only limit.
> Function Sequence::getNext is part of the HylaFAX source code.
> In util/Sequence.c++ you can see the real limit MAXSEQNUM
> which is defined as 999999999 in config.h[.in]

Right.

> If I understand the code correct, the highest possible value is  
> MAXSEQNUM-1. When the file contains this value, function getNext will  
> set the new value to 0 with NEXTSEQNUM, which is defined as (((x)+1) %  
> MAXSEQNUM). The next call to getNext will change this 0 to 1 and print  
> a warning.

Right.  I do think it's a good thing to get a warning when we've wrapped.

But if we *do* want to include MAXSEQNUM, then we just need to change
the check:
	seqnum >= MAXSEQNUM
to
	seqnum > MAXSEQNUM

> In order to avoid this warning and to allow MAXSEQNUM as maximum value  
> the macro could be changed to
> #define        NEXTSEQNUM(x)   (((x) % MAXSEQNUM) + 1)

The mod operation is actually redundant, since we have already checked
and made sure that seqnum is in our acceptable range.

This could safely be changed to:
#define        NEXTSEQNUM(x)   ((x) + 1)

When we store the "next" sequence number as MAXSEQNUM+1, it will be
caught next time, and a warning printed that we are rolling over.

Would the following patch make people happy?  I haven't tested this well
yet, but it's intended behaviour is:

1) Allow MAXSEQNUM (999999999) be used as a sequence value
2) Print a warning when we "overshoot" it, and reset it to 1
3) Remove a call to 64-bit mod
4) Never have 0 in the seqf file

A possible enhancement is a "wrapping" warning message instead of the
current "Invalid sequence number..." warning message.

===== config.h.in 1.12 vs edited =====
--- 1.12/config.h.in    2005-09-27 14:43:19 -04:00
+++ edited/config.h.in  2005-11-23 10:30:02 -05:00
@@ -283,7 +283,7 @@
  * numbers to be 16-bit values.
  */
 #define        MAXSEQNUM       999999999
-#define        NEXTSEQNUM(x)   (((x)+1) % MAXSEQNUM)
+#define        NEXTSEQNUM(x)   ((x)+1)

 /*
  * PAM Authentication
===== util/Sequence.c++ 1.2 vs edited =====
--- 1.2/util/Sequence.c++       2004-05-17 14:14:38 -04:00
+++ edited/util/Sequence.c++    2005-11-23 10:29:22 -05:00
@@ -72,7 +72,7 @@ u_long Sequence::getNext(const char* nam
     if (n > 0) {
         seqnum = atol(line);
     }
-    if (seqnum < 1 || seqnum >= MAXSEQNUM) {
+    if (seqnum < 1 || seqnum > MAXSEQNUM) {
         logWarning("%s: Invalid sequence number \"%s\", resetting to 1",
             name, line);
         seqnum = 1;




-- 
Aidan Van Dyk                                             aidan@xxxxxxxx
Senior Software Developer                          +1 215 438-4638 x8103
iFAX Solutions, Inc.                                http://www.ifax.com/

Attachment: signature.asc
Description: Digital signature