On Fri, Dec 27, 2019 at 09:15:33PM -0800, Steve Kargl wrote:
> On Fri, Dec 27, 2019 at 08:42:53PM -0800, Rodney W. Grimes wrote:
> > > On Fri, Dec 27, 2019 at 07:00:04PM -0800, Rodney W. Grimes wrote:
> > > > > On Fri, Dec 27, 2019 at 01:47:17PM -0800, Steve Kargl wrote:
> > > > >  
> > > > > This patch now includes a fix for hexadecimal conversion.  It
> > > > > simple scans the string for a hex digit in [a,...,f] and assumes
> > > > > that a hexadecimal string has been entered.  A string that includes
> > > > > character from the decimal digits is assumed to by a decimal
> > > > > representation.  
> > > > 
> > > > It looks to me that the old code did the common method of
> > > > try to convert as decimal, if that fails, try it as hex,
> > > > if that fails report an error.
> > > > 
> > > > Why is is that this common logic no longer works?
> > > 
> > > AFAICT, BN_dec2bn and BN_hex2bn from OpenSSL scan from left
> > > to right, does a conversion with what is possible, and reports
> > > success.  That is, for 1abc, BN_dec2bn can convert 1 to 1 and
> > > reports success.  The local implementations of these functions,
> > > when OpenSSL is not used, does not do this partial conversion.
> > 
> > I think I see now, the local implementaton checks for whole
> > string conversion with a test for newline or null as the last
> > byte converted by strtoul, the OpenSSL does not do this.
> > 
> 
> My patch fixes that.  The manpage documents that '1abcp' should
> convert '1abc'.  The 'p' simply terminates the conversion.  The
> local implementations actually flags an error.  I suspect the
> logic never worked as intended.  The use of OpenSSL functions 
> in factor(6) was introduced in r104722 by fanf@.
> 
> > So why ever use the, um, IMHO broken for this application,
> > SSL versions of these functions?  Or if we do need to use
> > them for some reason apply the whole string conversion
> > checks as wrappers around them?
> 
> You'll need to ask fanf@, but I suspect the SSL version 
> was introduced to allow the factoring of integers that
> exceed (uint64_t)(-1).
> 

Updated patch with a svn log message.

* usr.bin/factor/factor.6:
  . Document support for hexadecimal numbers.
  . Document termination conditions for interactive input.
  . Correct the maximum value for 'stop'.

* usr.bin/factor/factor.c:
  . Include stdbool for bool type.
  . New function, contains_hex_alpha_digits(), checks whether an string
    of digits contains one of the alpha digits for hexadecimal representations
    (ie., abcdef).  This function determines if decimal or hexadecimal 
    conversion is required.
  . In the WITHOUT_OPENSSL case, make BN_dec2bn() and BN_hex2bn() conform
    to the documented termination conditions for parsing a string.

* usr.bin/primes/primes.c:
  . Fix a comment, which has been wrong since 2014-09-27 (r272207).

Index: usr.bin/factor/factor.6
===================================================================
--- usr.bin/factor/factor.6     (revision 355983)
+++ usr.bin/factor/factor.6     (working copy)
@@ -36,7 +36,7 @@
 .\"
 .\"   chongo <for a good prime call: 391581 * 2^216193 - 1> /\oo/\
 .\"
-.Dd October 10, 2002
+.Dd December 27, 2019
 .Dt FACTOR 6
 .Os
 .Sh NAME
@@ -67,11 +67,20 @@
 .Nm
 is invoked with no arguments,
 .Nm
-reads numbers, one per line, from standard input, until end of file or error.
+reads numbers, one per line, from standard input until end of file or 0
+is entered or an error occurs.
 Leading white-space and empty lines are ignored.
 Numbers may be preceded by a single
 .Ql + .
 Numbers are terminated by a non-digit character (such as a newline).
+Numbers can be either decimal or hexadecimal strings.
+If the string contains only decimal digits, it is treated as a
+decimal representation for a number.
+A hexadecimal string should not contain a
+.Em 0x
+or
+.Em 0X
+prefix.
 After a number is read, it is factored.
 .Pp
 The
@@ -89,7 +98,7 @@
 value must not be greater than the maximum.
 The default and maximum value of
 .Ar stop
-is 3825123056546413050.
+is 18446744073709551615.
 .Pp
 When the
 .Nm primes
Index: usr.bin/factor/factor.c
===================================================================
--- usr.bin/factor/factor.c     (revision 355983)
+++ usr.bin/factor/factor.c     (working copy)
@@ -71,6 +71,7 @@
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -104,6 +105,7 @@
 
 #endif
 
+static bool    contains_hex_alpha_digits(char *str);
 static void    BN_print_dec_fp(FILE *, const BIGNUM *);
 
 static void    pr_fact(BIGNUM *);      /* print factors of a value */
@@ -148,21 +150,25 @@
                        for (p = buf; isblank(*p); ++p);
                        if (*p == '\n' || *p == '\0')
                                continue;
+                       if (*p == '+') p++;
                        if (*p == '-')
                                errx(1, "negative numbers aren't permitted.");
-                       if (BN_dec2bn(&val, buf) == 0 &&
-                           BN_hex2bn(&val, buf) == 0)
-                               errx(1, "%s: illegal numeric format.", buf);
+                       ch = contains_hex_alpha_digits(p) ?
+                           BN_hex2bn(&val, p) : BN_dec2bn(&val, p);
+                       if (ch == 0)
+                               errx(1, "%s: illegal numeric format.", p);
                        pr_fact(val);
                }
        /* Factor the arguments. */
        else
-               for (; *argv != NULL; ++argv) {
-                       if (argv[0][0] == '-')
+               for (p = *argv; p != NULL; p = *++argv) {
+                       if (*p == '+') p++;
+                       if (*p == '-')
                                errx(1, "negative numbers aren't permitted.");
-                       if (BN_dec2bn(&val, argv[0]) == 0 &&
-                           BN_hex2bn(&val, argv[0]) == 0)
-                               errx(1, "%s: illegal numeric format.", argv[0]);
+                       ch = contains_hex_alpha_digits(p) ?
+                           BN_hex2bn(&val, p) : BN_dec2bn(&val, p);
+                       if (ch == 0)
+                               errx(1, "%s: illegal numeric format.", p);
                        pr_fact(val);
                }
        exit(0);
@@ -346,7 +352,7 @@
 
        errno = 0;
        **a = strtoul(str, &p, 10);
-       return (errno == 0 && (*p == '\n' || *p == '\0'));
+       return (errno == 0 ? 1 : 0);    /* OpenSSL returns 0 on error! */
 }
 
 static int
@@ -356,7 +362,7 @@
 
        errno = 0;
        **a = strtoul(str, &p, 16);
-       return (errno == 0 && (*p == '\n' || *p == '\0'));
+       return (errno == 0 ? 1 : 0);    /* OpenSSL returns 0 on error! */
 }
 
 static BN_ULONG
@@ -370,3 +376,17 @@
 }
 
 #endif
+
+/* Check if the string contains a hexadecimal digit. */
+static bool
+contains_hex_alpha_digits(char *str)
+{
+       char c, *p;
+
+       for (p = str; *p; p++) {
+               c = tolower(*p);
+               if (c >= 'a' && c <= 'f')
+                       return true;
+       }
+       return false;
+}
Index: usr.bin/primes/primes.c
===================================================================
--- usr.bin/primes/primes.c     (revision 355983)
+++ usr.bin/primes/primes.c     (working copy)
@@ -55,7 +55,7 @@
  *     primes [-h] [start [stop]]
  *
  *     Print primes >= start and < stop.  If stop is omitted,
- *     the value 4294967295 (2^32-1) is assumed.  If start is
+ *     the value 18446744073709551615 (2^64-1) is assumed.  If start is
  *     omitted, start is read from standard input.
  *
  * validation check: there are 664579 primes between 0 and 10^7
-- 
Steve
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to