Re: [pdf-devel] Issues with bug FS#105

[gerel]

 > Date: Wed, 31 Mar 2010 14:04:19 +0200 (CEST)
 > From: Jose E. Marchesi <address@hidden>
 > 
 > 
 >     However, I believe there is some issue with signs.
 > 
 >     [...]
 > 
 >     That means that the result sign is _only_ decided on the dividend
 >     sign.
 > 
 > The sign of the integer modulus when either the dividend or the
 > divisor is negative is implementation defined in C.  Other languages,
 > such as Ada, enforce some policy or provide several different
 > solutions.
 > 
 > I don't have the Knuth books at hand now, but if we are implementing
 > that method then we have to use his rule of election.  Can someone
 > check?
 > 

Well, I have read a little about this issue. Our i64 implementation defines the
quotient as truncate(a/b), like many C implementations. For that reason the
remainder has the same sign as the dividend (like C99). I wrote a patch to fix
the mod result sign considering that and some tests to prove it.

OTOH, could someone test this patch on a win32 platform and evaluate the results
as for FS#105 ? I believe they are related.

Patch follows,

##
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: address@hidden
# target_branch: file:///home/gerel/PROJECTS/libgnupdf/trunk/
# testament_sha1: e103c25a94ba4c997041483413ae7b134bf36b50
# timestamp: 2010-04-07 11:58:22 -0300
# base_revision_id: address@hidden
# 
# Begin patch
=== modified file 'ChangeLog'
--- ChangeLog   2010-04-06 16:08:27 +0000
+++ ChangeLog   2010-04-07 14:57:19 +0000
@@ -1,3 +1,16 @@
+2010-04-07  Gerardo E. Gidoni  <address@hidden>
+
+       pdf_i64_mod: fixed the result sign to match with pdf_i64_div()
+
+       * src/base/pdf-types.c: although knuth defines the quotient as
+       floor(a/b), our implementation computes it as trunc(a/b), like C99,
+       the sign depends on the dividend.
+
+       * torture/unit/base/types/pdf-i64-div.c: added some tests to prove
+       the above assumption.
+
+       * torture/unit/base/types/pdf-i64-mod.c: likewise.
+
 2010-04-06  Jose E. Marchesi  <address@hidden>
 
        pdf-types: Define maximum integral values in terms of system

=== modified file 'src/base/pdf-types.c'
--- src/base/pdf-types.c        2010-02-20 16:02:07 +0000
+++ src/base/pdf-types.c        2010-04-07 14:57:19 +0000
@@ -885,20 +885,12 @@
       /*Now check the signs fo divisor and dividend*/
       if (pdf_i64_cmp(divisor,zero) < 0)
         {
-          result_sign = -1;
           pdf_i64_abs(&divisor,divisor, p_status);
         }
       if (pdf_i64_cmp(dividend,zero) < 0)
         {
           pdf_i64_abs(&dividend,dividend, p_status);
-          if (result_sign == -1)
-            {
-              result_sign = -1;
-            }
-          else
-            {
-              result_sign = -1;
-            }
+          result_sign = -1;
         }
 
       /*We check if abs(divisor) > abs(dividend)*/

=== modified file 'torture/unit/base/types/pdf-i64-div.c'
--- torture/unit/base/types/pdf-i64-div.c       2010-03-30 13:24:41 +0000
+++ torture/unit/base/types/pdf-i64-div.c       2010-04-07 14:57:19 +0000
@@ -198,6 +198,108 @@
 END_TEST
 
 
+/*
+ * Test: pdf_i64_div_006
+ * Description:
+ *   Divs a positive and negative number
+ * Success conditions:
+ * The call should not produce an error
+ */
+START_TEST(pdf_i64_div_006)
+{
+  pdf_status_t p_status = PDF_OK;
+  pdf_i64_t div1, div2, dest;
+
+#ifndef PDF_USE_BUILTIN_64BIT_SUPPORT 
+  pdf_init();
+
+  div1 = pdf_i64_new(0,6); 
+  div2 = pdf_i64_new(0xFFFFFFFF,0xFFFFFFFD); /*-3*/
+
+  pdf_i64_div(&dest, div1, div2, &p_status);
+  
+  fail_if(p_status != PDF_OK);
+  
+  fail_if(dest.low != 0xFFFFFFFE); /*-2*/
+  fail_if(dest.high != 0xFFFFFFFF);
+#else
+  div1 = 6;
+  div2 = -3;
+  pdf_i64_div(&dest, div1, div2, &p_status);
+  fail_if(dest != -2);
+#endif
+} 
+END_TEST
+
+/*
+ * Test: pdf_i64_div_007
+ * Description:
+ *   Divs a positive and negative number
+ * Success conditions:
+ * The call should not produce an error
+ */
+START_TEST(pdf_i64_div_007)
+{
+  pdf_status_t p_status = PDF_OK;
+  pdf_i64_t div1, div2, dest;
+
+#ifndef PDF_USE_BUILTIN_64BIT_SUPPORT 
+  pdf_init();
+
+  div1 = pdf_i64_new(0xFFFFFFFF,0xFFFFFFFB); /*-5*/
+  div2 = pdf_i64_new(0,3);
+
+  pdf_i64_div(&dest, div1, div2, &p_status);
+  
+  fail_if(p_status != PDF_OK);
+  
+  fail_if(dest.low != 0xFFFFFFFF); /*-1*/
+  fail_if(dest.high != 0xFFFFFFFF);
+#else
+  div1 = -5;
+  div2 = 3;
+  pdf_i64_div(&dest, div1, div2, &p_status);
+  fail_if(dest != -1);
+#endif
+
+}
+END_TEST
+
+
+/*
+ * Test: pdf_i64_div_008
+ * Description:
+ *   Divs a positive and negative number
+ * Success conditions:
+ * The call should not produce an error
+ */
+START_TEST(pdf_i64_div_008)
+{
+  pdf_status_t p_status = PDF_OK;
+  pdf_i64_t div1, div2, dest;
+
+#ifndef PDF_USE_BUILTIN_64BIT_SUPPORT 
+  pdf_init();
+
+  div1 = pdf_i64_new(0,5);
+  div2 = pdf_i64_new(0xFFFFFFFF,0xFFFFFFFD); /*-3*/
+
+  pdf_i64_div(&dest, div1, div2, &p_status);
+  
+  fail_if(p_status != PDF_OK);
+  
+  fail_if(dest.low != 0xFFFFFFFF); /*-1*/
+  fail_if(dest.high != 0xFFFFFFFF);
+#else
+  div1 = 5;
+  div2 = -3;
+  pdf_i64_div(&dest, div1, div2, &p_status);
+  fail_if(dest != -1);
+#endif
+
+}
+END_TEST
+
 
 
 /*
@@ -212,6 +314,9 @@
   tcase_add_test(tc, pdf_i64_div_003);
   tcase_add_test(tc, pdf_i64_div_004);
   tcase_add_test(tc, pdf_i64_div_005);
+  tcase_add_test(tc, pdf_i64_div_006);
+  tcase_add_test(tc, pdf_i64_div_007);
+  tcase_add_test(tc, pdf_i64_div_008);
   return tc;
 }
 

=== modified file 'torture/unit/base/types/pdf-i64-mod.c'
--- torture/unit/base/types/pdf-i64-mod.c       2010-03-30 13:24:41 +0000
+++ torture/unit/base/types/pdf-i64-mod.c       2010-04-07 14:57:19 +0000
@@ -192,6 +192,74 @@
 END_TEST
 
 /*
+ * Test: pdf_i64_mod_006
+ * Description:
+ *   Mods a positive and negative number
+ * Success conditions:
+ * The call should not produce an error
+ */
+START_TEST(pdf_i64_mod_006)
+{
+  pdf_status_t p_status = PDF_OK;
+  pdf_i64_t mod1, mod2, dest;
+
+#ifndef PDF_USE_BUILTIN_64BIT_SUPPORT 
+  pdf_init();
+
+  mod1 = pdf_i64_new(0,5);
+  mod2 = pdf_i64_new(0xFFFFFFFF,0xFFFFFFFD); /*-3*/
+
+  pdf_i64_mod(&dest, mod1, mod2, &p_status);
+  
+  fail_if(p_status != PDF_OK);
+
+  fail_if(dest.low != 2); 
+  fail_if(dest.high != 0);
+#else
+  mod1 = 5;
+  mod2 = -3;
+  pdf_i64_mod(&dest, mod1, mod2, &p_status);
+  fail_if(dest != 2);
+#endif
+}
+END_TEST
+
+
+/*
+ * Test: pdf_i64_mod_007
+ * Description:
+ *   Mods a positive and negative number
+ * Success conditions:
+ * The call should not produce an error
+ */
+START_TEST(pdf_i64_mod_007)
+{
+  pdf_status_t p_status = PDF_OK;
+  pdf_i64_t mod1, mod2, dest;
+
+#ifndef PDF_USE_BUILTIN_64BIT_SUPPORT 
+  pdf_init();
+
+  mod1 = pdf_i64_new(0xFFFFFFFF,0xFFFFFFFB); /*-5*/
+  mod2 = pdf_i64_new(0,3);
+
+  pdf_i64_mod(&dest, mod1, mod2, &p_status);
+  
+  fail_if(p_status != PDF_OK);
+
+  fail_if(dest.low !=  0xFFFFFFFE);
+  fail_if(dest.high != 0xFFFFFFFF);
+#else
+  mod1 = -5;
+  mod2 = 3;
+  pdf_i64_mod(&dest, mod1, mod2, &p_status);
+  fail_if(dest != -2);
+#endif
+}
+END_TEST
+
+
+/*
  * Test case creation function
  */
 TCase *
@@ -203,6 +271,8 @@
   tcase_add_test(tc, pdf_i64_mod_003);
   tcase_add_test(tc, pdf_i64_mod_004);
   tcase_add_test(tc, pdf_i64_mod_005);
+  tcase_add_test(tc, pdf_i64_mod_006);
+  tcase_add_test(tc, pdf_i64_mod_007);
   return tc;
 }
 

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWQ6HfBsABirfgA0weXf//3+v
3gC////6YAo9523vsgFHnswUO5O10BINdCm2AhxkyaMQxNMBAwJpgjBMTTTQAYQSiJkGQaCaT1J5
NQ2iPRAANAAAGg1U9pPRQaAAZNAAAAAAAAASaoiFB6n5FP0mo9NQ9QM1DRo0eoABoANAikKn6J6Q
mmp6TI9TTRpkaA00yNDQAMjJ6gRSEAhomamEyamaEp6YahpkhmmoGmRp6JwEDFiIfZ9V92c7XllH
sybyqwyVTXqaY9NMRWPLpugNzdlzlQJZnEzcmeQxJlObEJQkTKRJhghaJqTvTMI303sfglLoTO0j
WSaVEwG+HFlnnYb4Xmb+sJXoQBJ1AgmpL28y1tiK6bCtDU1VWMhA6kLqWgLoht+VkzJaSAfz9By0
QSjshsE0VFbCZQPEH1DaYgiIGIKdOSS64ZK2npQkkAgYICWwZJhylpYadg5izURhSyeGGkPUM9oC
nRiCHYkiSQ9+5LHpHhBmE5JUrclnQgTIgcdE7ktKWGBqyGSFaJMEkFx0auD9YYWuzFgmGMkEh3Av
ExpipVLVC4bFqxERMJSiNIk1fzS1M2DcmSSoZ9VqUrA1Y1kiVZGgsBNnLjx3FI/Tmyq8JcYD9OMd
24aQCdFfeJiJ9ByCNyR5CY886tkJiliXdD1ZRGoo/Mg9JSujwTfCfKV1cfYG7adjSKFnyfxNHmmx
B+DIDU6MxhKtOU1dg1IfXIJDLemKah5+gPoYHQ89oTqIKSLj07yQTbIV0i1uktYkSLsDFl2h5dVC
o8sn22HM8btfb16svGyRIkRKWIm0E1o+SbAfWb6zLHq0hmIowGNwBEJOOaSluJPIhQ/4DMhhDZ0S
BKHCDG2TlLSb9zjqkBzwAUJ0MCpU2hxgGQ8gpBIugLaEigNoHXNshCbf6/Pme1uG1wCzgdthhXpg
PqPNNoG5NZl9CyoIhu780RBEEWFkFiQFEKDi6SoNdoJe6TXmkUESGBDe0ITxS84T0YpGsjx5Krtk
RxUhcaE1mgjpgT+2Z300FWl3kqBqeGloJYa56VzuQeSMTYcWmeAVTAOY38t+hvwrNZrVUb8ZjVqW
QVYl0sLInlChdelLusyNbNp2RuifC2Rcbryor4w35bSeg7KFufKa5ErSwHVvBvsubZcyx1NRXMtl
2FhxhsHGB2ZgZsCJS2wGQFV+IuonLBllBWZdg7gY1kIZqxlttU4KGsUEeJI4mcAaFA8HQonBOQ2r
QoOImIpPUUGCDUAS0beZkx/jMqak+ZgU3vsrY6ajQyNY4pUVUHWkcTERNLotC2KrYTkDEVG2MBLo
pyg0vlJoXtxFwmCnKx24JZb0GoAltO/92ylxwj4wlMTFgisuNTuka9stZKuYbQpK8gkpSKATgmIs
w0LmMcaEAutEpNB1BY66dKd0JTmbSQogxEgMCwqLxpOSky9KlpOa6N29MCFp5wS5K6MpPGglttb3
D9uqlR5SNISyR7olWJ9NXIZEXk5ruKyRAYLUcDpQVCYvF7yD5RYEoMqsmzSZXbCcTSshvlWKpNbr
440LJBdAFrRJYHNDdCkkOINQ/CURgkGDBkmTnYQz1VcApWMNvd8wU26hiSPoh7zJ0hRBXTDvQ3Zs
dz1K/xKATIxddkwE/qW+wo8gh8VcRqS5OJNQnWmSpZJpnEy7CXVVklktXWTqSiXCaUqS1NKXpbIS
gn0TZnStLBMUsTHDk31VVEN6GWGgg5oYQYVhhDTAOIFOZPthkcQSiTWoU9nKtD5AlaVJUmKGCHuG
UCHDpOiPQcfmmHlZzsOpG9cxY6eUONBzKiN57j79n5LiCfh9yYcA3ySuE2t2SU1KVHwC5JVJb7kh
B8PefWeZIug4Gs+B1lR10OVzz4GwsMCvjMvLjUnw0F1nuF+bw2JId+JnNJxO3DIrOB1WlXoMizYd
5oeKaU3n5CWxnTUY1mvWno7Ozqnu0D3Jfcnikn+oTgOECdlchLtpxLd5cNo5S3YF5ehA7E00qLiY
0qsTgabor8k/YSBJp4DQ7CQKytSSQnkB+NRFYrNCP5PZhnLeJOYwe7eny4bkJhGCbAYgwOd5MCpA
z1G7dplyvi3g4oOI2moLQ27lM1GUyQvBiJjdjzF5WVP3d0m7fVjQrwILsHhZq+qPL9ZFQB1H7J/Z
MswOaieXfanDmyGvtSOPL99CB208UzS65AfamjkmOZLEgE+1ISc8RLhPKaFLU606SBLR+sg+ppx4
0LDft6FkEj/POwUfE7twJ1vpM96Z+hidSYB9A8wwCyDaH62Jvoev1F5Li+3w5EMiYhkEGdOGELBa
/Ye5ICmcuhyIcwQ2cMO2p1pfcFCcJD6ULeCDDLuhtnYnVxEy3VHodIe8FO4M2kvqqQ9nsneXmhKw
SE86ciZg4jugmuDebxvrPRgnLu6KX4REJCQmxPXsXNkszqSxNclBtEjzcSoOrk6E+4Hkdx7QyXT7
O29PzZBnDOtJ7GFC8E8UgH7bziO+rIsMO9JJRws75G1qTsMCLE22hVtxK3uOFnqNoU2kZgSYE8L0
vzXZgZ+U4rSUiEbgrQa1XOkkqkVSVLCRVnhNLyqUS5dJDVA6DAErTuqc2rJ94PS5EP4Mh+e03cXl
6DRphGSe3GaW3rlpXkCdTzmkJ4JVv2Dou9+CZelLYekg7EEnHxqtYFqKLDyW0qikJZaxEXJu5ekC
excmaEtFNlI5geVGHmdor5nImRMPTKbC5S7aDUTQJKWH8Y+yaepvBPiFvr6JR9xTrTurlzSKDW1Q
6MRPF8DqSyUWV1iTAodEqoldQ96WcYA2HVAYhrSIal0JMMBz1F43QglSDasgwlCUz31iXpmJpWrV
AkiBK5h7AqgKThB7kwSsoVZ/jpzJYhfPA6UCKX5zZiT2DaJJPvLnUsGkIlbMa5FaD0Mu0jwLBTmH
zTeOkbMzWkYWnZOvao6rMRE16CSFYSIT0azLSvucrNqlBOAHj4Z7Rsmg1j1CEGI8YPKXobToA5NO
23p5Au4RP+LuSKcKEgHQ74Ng

##


-gerel