Re: OpenSSL breaks factor(6)
On Sun, Dec 29, 2019 at 08:02:47AM -0800, Steve Kargl wrote: > Here's a final attempt at fixing and documenting FreeBSD's factor(6). > Do what you want with the patch. With and without OpenSSL, one now > gets > > % factor +123 123 123 123zabc 123abc +123abc 0x123abc +0x123abc > 123: 3 41 > 123: 3 41 > 123: 3 41 > 123: 3 41 > 1194684: 2 2 3 29 3433 > 1194684: 2 2 3 29 3433 > 1194684: 2 2 3 29 3433 > 1194684: 2 2 3 29 3433 > > * usr.bin/factor/factor.6: > . Update documentation to note that hexadecimal strings are accepted. > . Document that a hexadecimal number can have an optional 0x or 0X prefix. > . Document that a 0 value in interactive mode terminates factor(6). > . Fix the maximum value for 'stop' in primes(6). > . While here, spell "white-space" as "whitespace" and "non-digit" as > "nondigit". > > * usr.bin/factor/factor.c: > . Include stdbool to get acces to bool type. > . Use consistent style for function prototypes. > . New function. is_hex_str() looks for the longest substring and > determines if it is a hexadecimal number. > . New function. Factor (pun intended) out common code into > convert_str2bn(). > . For the WIHTOUT_OPENSSL case, make BN_dec2bn() and BN_hex2bn() return 0 > on error like their OpenSSL counterparts. > > * usr.bin/primes/primes.c: > . Fix comment. > This is now https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=243136 -- 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"
Re: OpenSSL breaks factor(6)
On Sun, Dec 29, 2019 at 05:14:28PM -0500, Garance A Drosehn wrote: > On 29 Dec 2019, at 2:17, Steve Kargl wrote: > > > On Sun, Dec 29, 2019 at 01:34:28AM -0500, Garance A Drosehn wrote: > >>> > >>> An interested user will need to add that support. AFAIK, factor(6) > >>> has never recognized the 0x prefix, and I'm not trying to add new > >>> features. I'm simply fixing factor(6) to match its documentation, > >>> and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the > >>> same results where applicable. > >> > >> Well, I'd be willing to do the work to add the new feature, and also > >> make the commit. It'd be a nice easy task for me to tackle... :) > >> > >> But I think documenting that "hex works, but only for hex values > >> which have at least one A-F in the value" is not something that I'd > >> want to draw attention to via documentation. > >> > > > > You have a 17 year history to worry about as hexadecimal support > > was added by "r104720 | fanf | 2002-10-09". Compiling factor(6) > > with and without OpenSSL support after 2002-10-09 gives a utility > > with different inconsistent behavior. > > If I understand you right, that behavior has not been documented > for 17 years. If it continues to be un-documented, that cannot > possibly break any scripts. I'm not saying we should remove the > behavior, I'm just saying we don't need to document it. Especially > not if we add support for a better way to specify hex values. See the most recent patch. It does everything you want with '0x' or '0X', maintains what was likely the intended behavior, and makes the with/without OpenSSL support consistent. I disagree with not documenting what the utility does. Currently, the string '1abc' returns '1' with OpenSSL and 6844 without OpenSSL. It is not possible to understand this behavior based on the manpage. > > On 29 Dec 2019, at 1:50, Steve Kargl wrote: > > > >Do what you want with the patch (including ignoring it). > >Hopefully, someone in the FreeBSD project will now > >recognize that factor(6) with and without OpenSSL gives > >inconsistent results, and neither matches factor(6)'s > >manpage. > > Oh. I tend to lose track with who is and isn't a src-committer on > FreeBSD. I've seen your name enough that I assumed you were one. > If you're not, I can handle committing these changes, including the > new feature. That'll keep my commit bit alive for another year! > I gave up my commit bit years ago, because the individuals who set up jenkins reduced freebsd-current@ to a spam repository for jenkins build logs and refused to stop spamming the list. freebsd-current@ has yet to recover from that era. I still have my ka...@freebsd.org address. I do, however, still use FreeBSD. When I find a problem and can fix it, I submit patches to the relevant mailing list and/or bugzilla. -- 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"
Re: OpenSSL breaks factor(6)
On Sun, Dec 29, 2019 at 01:31:26PM -0900, Robert Wing wrote: > Have y'all ever seen reviews.freebsd.org? > I have a FreeBSD login account and a FreeBSD bugzilla account. I don't need yet another FreeBSD account. If bugzilla were set up to parse email replies, this would be attached a bug report. -- 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"
Re: OpenSSL breaks factor(6)
Have y'all ever seen reviews.freebsd.org? On Sunday, December 29, 2019, Garance A Drosehn wrote: > On 29 Dec 2019, at 2:17, Steve Kargl wrote: > > > On Sun, Dec 29, 2019 at 01:34:28AM -0500, Garance A Drosehn wrote: > >>> > >>> An interested user will need to add that support. AFAIK, factor(6) > >>> has never recognized the 0x prefix, and I'm not trying to add new > >>> features. I'm simply fixing factor(6) to match its documentation, > >>> and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the > >>> same results where applicable. > >> > >> Well, I'd be willing to do the work to add the new feature, and also > >> make the commit. It'd be a nice easy task for me to tackle... :) > >> > >> But I think documenting that "hex works, but only for hex values > >> which have at least one A-F in the value" is not something that I'd > >> want to draw attention to via documentation. > >> > > > > You have a 17 year history to worry about as hexadecimal support > > was added by "r104720 | fanf | 2002-10-09". Compiling factor(6) > > with and without OpenSSL support after 2002-10-09 gives a utility > > with different inconsistent behavior. > > If I understand you right, that behavior has not been documented > for 17 years. If it continues to be un-documented, that cannot > possibly break any scripts. I'm not saying we should remove the > behavior, I'm just saying we don't need to document it. Especially > not if we add support for a better way to specify hex values. > > On 29 Dec 2019, at 1:50, Steve Kargl wrote: > > > >Do what you want with the patch (including ignoring it). > >Hopefully, someone in the FreeBSD project will now > >recognize that factor(6) with and without OpenSSL gives > >inconsistent results, and neither matches factor(6)'s > >manpage. > > Oh. I tend to lose track with who is and isn't a src-committer on > FreeBSD. I've seen your name enough that I assumed you were one. > If you're not, I can handle committing these changes, including the > new feature. That'll keep my commit bit alive for another year! > > -- > Garance Alistair Drosehn = dro...@rpi.edu or g...@freebsd.org > ___ > 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" > ___ 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"
Re: OpenSSL breaks factor(6)
On 29 Dec 2019, at 2:17, Steve Kargl wrote: > On Sun, Dec 29, 2019 at 01:34:28AM -0500, Garance A Drosehn wrote: >>> >>> An interested user will need to add that support. AFAIK, factor(6) >>> has never recognized the 0x prefix, and I'm not trying to add new >>> features. I'm simply fixing factor(6) to match its documentation, >>> and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the >>> same results where applicable. >> >> Well, I'd be willing to do the work to add the new feature, and also >> make the commit. It'd be a nice easy task for me to tackle... :) >> >> But I think documenting that "hex works, but only for hex values >> which have at least one A-F in the value" is not something that I'd >> want to draw attention to via documentation. >> > > You have a 17 year history to worry about as hexadecimal support > was added by "r104720 | fanf | 2002-10-09". Compiling factor(6) > with and without OpenSSL support after 2002-10-09 gives a utility > with different inconsistent behavior. If I understand you right, that behavior has not been documented for 17 years. If it continues to be un-documented, that cannot possibly break any scripts. I'm not saying we should remove the behavior, I'm just saying we don't need to document it. Especially not if we add support for a better way to specify hex values. On 29 Dec 2019, at 1:50, Steve Kargl wrote: > >Do what you want with the patch (including ignoring it). >Hopefully, someone in the FreeBSD project will now >recognize that factor(6) with and without OpenSSL gives >inconsistent results, and neither matches factor(6)'s >manpage. Oh. I tend to lose track with who is and isn't a src-committer on FreeBSD. I've seen your name enough that I assumed you were one. If you're not, I can handle committing these changes, including the new feature. That'll keep my commit bit alive for another year! -- Garance Alistair Drosehn = dro...@rpi.edu or g...@freebsd.org ___ 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"
Re: OpenSSL breaks factor(6)
On Sun, Dec 29, 2019 at 08:02:47AM -0800, Steve Kargl wrote: > Here's a final attempt at fixing and documenting FreeBSD's factor(6). > Do what you want with the patch. With and without OpenSSL, one now > gets > > % factor +123 123 123 123zabc 123abc +123abc 0x123abc +0x123abc > 123: 3 41 > 123: 3 41 > 123: 3 41 > 123: 3 41 > 1194684: 2 2 3 29 3433 > 1194684: 2 2 3 29 3433 > 1194684: 2 2 3 29 3433 > 1194684: 2 2 3 29 3433 > > * usr.bin/factor/factor.6: > . Update documentation to note that hexadecimal strings are accepted. > . Document that a hexadecimal number can have an optional 0x or 0X prefix. > . Document that a 0 value in interactive mode terminates factor(6). Whoops. Leading zeros are ignored. > . Fix the maximum value for 'stop' in primes(6). > . While here, spell "white-space" as "whitespace" and "non-digit" as > "nondigit". -- 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"
Re: OpenSSL breaks factor(6)
Here's a final attempt at fixing and documenting FreeBSD's factor(6).
Do what you want with the patch. With and without OpenSSL, one now
gets
% factor +123 123 123 123zabc 123abc +123abc 0x123abc +0x123abc
123: 3 41
123: 3 41
123: 3 41
123: 3 41
1194684: 2 2 3 29 3433
1194684: 2 2 3 29 3433
1194684: 2 2 3 29 3433
1194684: 2 2 3 29 3433
* usr.bin/factor/factor.6:
. Update documentation to note that hexadecimal strings are accepted.
. Document that a hexadecimal number can have an optional 0x or 0X prefix.
. Document that a 0 value in interactive mode terminates factor(6).
. Fix the maximum value for 'stop' in primes(6).
. While here, spell "white-space" as "whitespace" and "non-digit" as
"nondigit".
* usr.bin/factor/factor.c:
. Include stdbool to get acces to bool type.
. Use consistent style for function prototypes.
. New function. is_hex_str() looks for the longest substring and
determines if it is a hexadecimal number.
. New function. Factor (pun intended) out common code into convert_str2bn().
. For the WIHTOUT_OPENSSL case, make BN_dec2bn() and BN_hex2bn() return 0
on error like their OpenSSL counterparts.
* usr.bin/primes/primes.c:
. Fix comment.
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 /\oo/\
.\"
-.Dd October 10, 2002
+.Dd December 29, 2019
.Dt FACTOR 6
.Os
.Sh NAME
@@ -67,11 +67,22 @@
.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.
+.Pp
Numbers may be preceded by a single
.Ql + .
+Numbers can be either decimal or hexadecimal strings where the longest
+leading substring is used.
Numbers are terminated by a non-digit character (such as a newline).
+If the string contains only decimal digits, it is treated as a
+decimal representation for a number.
+A hexadecimal string can contain an optional
+.Em 0x
+or
+.Em 0X
+prefix.
After a number is read, it is factored.
.Pp
The
@@ -89,7 +100,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
#include
#include
+#include
#include
#include
#include
@@ -97,8 +98,8 @@
#define BN_is_one(v) (*(v) == 1)
#define BN_mod_word(a, b) (*(a) % (b))
-static int BN_dec2bn(BIGNUM **a, const char *str);
-static int BN_hex2bn(BIGNUM **a, const char *str);
+static int BN_dec2bn(BIGNUM **, const char *);
+static int BN_hex2bn(BIGNUM **, const char *);
static BN_ULONG BN_div_word(BIGNUM *, BN_ULONG);
static voidBN_print_fp(FILE *, const BIGNUM *);
@@ -105,7 +106,8 @@
#endif
static voidBN_print_dec_fp(FILE *, const BIGNUM *);
-
+static voidconvert_str2bn(BIGNUM **, char *);
+static boolis_hex_str(char *);
static voidpr_fact(BIGNUM *); /* print factors of a value */
static voidpr_print(BIGNUM *); /* print a prime */
static voidusage(void);
@@ -148,21 +150,13 @@
for (p = buf; isblank(*p); ++p);
if (*p == '\n' || *p == '\0')
continue;
- if (*p == '-')
- errx(1, "negative numbers aren't permitted.");
- if (BN_dec2bn(, buf) == 0 &&
- BN_hex2bn(, buf) == 0)
- errx(1, "%s: illegal numeric format.", buf);
+ convert_str2bn(, p);
pr_fact(val);
}
/* Factor the arguments. */
else
- for (; *argv != NULL; ++argv) {
- if (argv[0][0] == '-')
- errx(1, "negative numbers aren't permitted.");
- if (BN_dec2bn(, argv[0]) == 0 &&
- BN_hex2bn(, argv[0]) == 0)
- errx(1, "%s: illegal numeric format.", argv[0]);
+ for (p = *argv; p != NULL; p = *++argv) {
+ convert_str2bn(, p);
pr_fact(val);
}
exit(0);
@@ -346,7 +340,7 @@
errno = 0;
**a = strtoul(str, , 10);
- return (errno == 0 && (*p == '\n' || *p == '\0'));
+ return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
}
static int
@@ -356,7 +350,7 @@
errno = 0;
**a = strtoul(str, , 16);
-
Re: OpenSSL breaks factor(6)
On Sat, Dec 28, 2019 at 11:17:31PM -0800, Steve Kargl wrote:
> On Sun, Dec 29, 2019 at 01:34:28AM -0500, Garance A Drosehn wrote:
> > On 29 Dec 2019, at 0:10, Steve Kargl wrote:
> >
> > > On Sat, Dec 28, 2019 at 10:46:52PM -0500, Garance A Drosehn wrote:
> > >>
> > >> What if the user wants to factor a hexadecimal value which does not
> > >> happen to include [a...f]?
> > >>
> > >> How about recognizing a prefix of '0x' as a way to indicate the value
> > >> is hexadecimal?
Here you go. '0x' support is undocumented.
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 /\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
#include
#include
+#include
#include
#include
#include
@@ -104,6 +105,7 @@
#endif
+static boolcontains_hex_alpha_digits(char *str);
static voidBN_print_dec_fp(FILE *, const BIGNUM *);
static voidpr_fact(BIGNUM *); /* print factors of a value */
@@ -148,21 +150,37 @@
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(, buf) == 0 &&
- BN_hex2bn(, buf) == 0)
- errx(1, "%s: illegal numeric format.", buf);
+ if (*p == '0') {
+ p++;
+ if (*p == 'x' || *p == 'X')
+ ch = BN_hex2bn(, ++p);
+ } else {
+ ch = contains_hex_alpha_digits(p) ?
+ BN_hex2bn(, p) : BN_dec2bn(, 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(, argv[0]) == 0 &&
- BN_hex2bn(, argv[0]) == 0)
- errx(1, "%s: illegal numeric format.", argv[0]);
+ if (*p == '0') {
+ p++;
+ if (*p == 'x' || *p == 'X')
+ ch = BN_hex2bn(, ++p);
+ } else {
+ ch = contains_hex_alpha_digits(p) ?
+ BN_hex2bn(, p) : BN_dec2bn(, p);
+ }
+ if (ch == 0)
+ errx(1, "%s: illegal numeric format.", p);
pr_fact(val);
}
exit(0);
@@ -346,7 +364,7 @@
errno = 0;
**a = strtoul(str, , 10);
- return (errno == 0 && (*p == '\n' || *p == '\0'));
+ return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
}
static int
@@ -356,7 +374,7 @@
errno = 0;
**a = strtoul(str, , 16);
- return (errno == 0 && (*p == '\n' || *p == '\0'));
+ return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
}
static BN_ULONG
@@ -370,3 +388,20 @@
}
#endif
+
+/*
+ * Check if the string contains one of 'abcdef' from
Re: OpenSSL breaks factor(6)
On Sun, Dec 29, 2019 at 01:34:28AM -0500, Garance A Drosehn wrote: > On 29 Dec 2019, at 0:10, Steve Kargl wrote: > > > On Sat, Dec 28, 2019 at 10:46:52PM -0500, Garance A Drosehn wrote: > >> > >> What if the user wants to factor a hexadecimal value which does not > >> happen to include [a...f]? > >> > >> How about recognizing a prefix of '0x' as a way to indicate the value > >> is hexadecimal? > > > > An interested user will need to add that support. AFAIK, factor(6) > > has never recognized the 0x prefix, and I'm not trying to add new > > features. I'm simply fixing factor(6) to match its documentation, > > and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the > > same results where applicable. > > Well, I'd be willing to do the work to add the new feature, and also > make the commit. It'd be a nice easy task for me to tackle... :) > > But I think documenting that "hex works, but only for hex values > which have at least one A-F in the value" is not something that I'd > want to draw attention to via documentation. > You have a 17 year history to worry about as hexadecimal support was added by "r104720 | fanf | 2002-10-09". Compiling factor(6) with and without OpenSSL support after 2002-10-09 gives a utility with different inconsistent behavior. Current code: With OpenSSL % factor 1abc 1: 1 % factor 1abc 1: 1 % factor +125 factor: +125: illegal numeric format. Without OpenSSL % factor 1abc 6844: 2 2 29 59 % factor 1abc factor: 1abc: illegal numeric format. % factor +125 125: 5 5 5 Patched code: With OpenSSL % factor 1abc 6844: 2 2 29 59 % factor 1abc 6844: 2 2 29 59 % factor +125 125: 5 5 5 Without OpenSSL % factor 1abc 6844: 2 2 29 59 % factor 1abc 6844: 2 2 29 59 % factor +125 125: 5 5 5 -- 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"
Re: OpenSSL breaks factor(6)
On Sat, Dec 28, 2019 at 09:34:52PM -0800, Rodney W. Grimes wrote: > > On Sat, Dec 28, 2019 at 10:46:52PM -0500, Garance A Drosehn wrote: > > > On 27 Dec 2019, at 17:42, 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. > > > > > > What if the user wants to factor a hexadecimal value which does not > > > happen to include [a...f]? > > > > > > How about recognizing a prefix of '0x' as a way to indicate the value > > > is hexadecimal? > > > > > > > An interested user will need to add that support. AFAIK, factor(6) > > has never recognized the 0x prefix, and I'm not trying to add new > > features. I'm simply fixing factor(6) to match its documentation, > > and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the > > same results where applicable. > > > > The logic is to first try to convert the string to a decimal if > > the leading digits are members of the set [0,...,9]. If this > > conversion fails, then try to convert the string as a hexadecimal > > number. A problem occurs because OpenSSL's BN_dec2bn does not fail > > for a number like '1abc' (converts it to 1) whereas the local > > implementation of BN_dec2bn fails during the conversion, and so > > the BN_hex2bn function is executed and '1abc' is converted. > > Wasnt the hex conversion undocumented? Yes, and I fixed the manpage to document the behavior. And, I fixed factor(6) to match the documentation in other aspects (e.g., leading '+' character). > Since it seems to have issues, and is of dubious value > might it might be best to just remove it? It has been a part of FreeBSD's factor since r104722 | fanf | 2002-10-09 That is 17 years. Are you sure no one is using this feature in some script? What about backwards compatibility? AFAICT, with my limited testing, my patch should fix the issues that I discovered. Do what you want with the patch (including ignoring it). Hopefully, someone in the FreeBSD project will now recognize that factor(6) with and without OpenSSL gives inconsistent results, and neither matches factor(6)'s manpage. -- 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"
Re: OpenSSL breaks factor(6)
On 29 Dec 2019, at 0:10, Steve Kargl wrote: > On Sat, Dec 28, 2019 at 10:46:52PM -0500, Garance A Drosehn wrote: >> >> What if the user wants to factor a hexadecimal value which does not >> happen to include [a...f]? >> >> How about recognizing a prefix of '0x' as a way to indicate the value >> is hexadecimal? > > An interested user will need to add that support. AFAIK, factor(6) > has never recognized the 0x prefix, and I'm not trying to add new > features. I'm simply fixing factor(6) to match its documentation, > and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the > same results where applicable. Well, I'd be willing to do the work to add the new feature, and also make the commit. It'd be a nice easy task for me to tackle... :) But I think documenting that "hex works, but only for hex values which have at least one A-F in the value" is not something that I'd want to draw attention to via documentation. -- Garance Alistair Drosehn = dro...@rpi.edu or g...@freebsd.org ___ 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"
Re: OpenSSL breaks factor(6)
> On Sat, Dec 28, 2019 at 10:46:52PM -0500, Garance A Drosehn wrote: > > On 27 Dec 2019, at 17:42, 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. > > > > What if the user wants to factor a hexadecimal value which does not > > happen to include [a...f]? > > > > How about recognizing a prefix of '0x' as a way to indicate the value > > is hexadecimal? > > > > An interested user will need to add that support. AFAIK, factor(6) > has never recognized the 0x prefix, and I'm not trying to add new > features. I'm simply fixing factor(6) to match its documentation, > and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the > same results where applicable. > > The logic is to first try to convert the string to a decimal if > the leading digits are members of the set [0,...,9]. If this > conversion fails, then try to convert the string as a hexadecimal > number. A problem occurs because OpenSSL's BN_dec2bn does not fail > for a number like '1abc' (converts it to 1) whereas the local > implementation of BN_dec2bn fails during the conversion, and so > the BN_hex2bn function is executed and '1abc' is converted. Wasnt the hex conversion undocumented? Since it seems to have issues, and is of dubious value might it might be best to just remove it? > -- > Steve -- Rod Grimes rgri...@freebsd.org ___ 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"
Re: OpenSSL breaks factor(6)
On Sat, Dec 28, 2019 at 10:46:52PM -0500, Garance A Drosehn wrote: > On 27 Dec 2019, at 17:42, 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. > > What if the user wants to factor a hexadecimal value which does not > happen to include [a...f]? > > How about recognizing a prefix of '0x' as a way to indicate the value > is hexadecimal? > An interested user will need to add that support. AFAIK, factor(6) has never recognized the 0x prefix, and I'm not trying to add new features. I'm simply fixing factor(6) to match its documentation, and trying to ensure WITH_OPENSSL and WITHOUT_OPENSSL give the same results where applicable. The logic is to first try to convert the string to a decimal if the leading digits are members of the set [0,...,9]. If this conversion fails, then try to convert the string as a hexadecimal number. A problem occurs because OpenSSL's BN_dec2bn does not fail for a number like '1abc' (converts it to 1) whereas the local implementation of BN_dec2bn fails during the conversion, and so the BN_hex2bn function is executed and '1abc' is converted. -- 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"
Re: OpenSSL breaks factor(6)
On 27 Dec 2019, at 17:42, 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. What if the user wants to factor a hexadecimal value which does not happen to include [a...f]? How about recognizing a prefix of '0x' as a way to indicate the value is hexadecimal? -- Garance Alistair Drosehn= dro...@rpi.edu Senior Systems Programmer or g...@freebsd.org Rensselaer Polytechnic Institute; Troy, NY; USA ___ 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"
Re: OpenSSL breaks factor(6)
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 /\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 #include #include +#include #include #include #include @@ -104,6 +105,7 @@ #endif +static boolcontains_hex_alpha_digits(char *str); static voidBN_print_dec_fp(FILE *, const BIGNUM *); static voidpr_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(, buf) == 0 && - BN_hex2bn(, buf) == 0)
Re: OpenSSL breaks factor(6)
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). -- 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"
Re: OpenSSL breaks factor(6)
> 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:
> > > > On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote:
> > > > > The use of OpenSSL in factor(6) breaks factor(6) with respect to
> > > > > its documentation.
> > > > >
> > > > > % man factor
> > > > > ...
> > > > > Numbers may be preceded by a single '+'.
> > > > > ...
> > > > >
> > > > > % factor +125
> > > > > factor: +125: illegal numeric format.
> > > > >
> > > >
> > > > This fixes factor(6) for the above issue. The issue with
> > > > hexadecimal is not easily fixed.
> > > >
> > >
> > > 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.
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?
> > >
> > > Index: factor.c
> > > ===
> > > --- factor.c (revision 355983)
> > > +++ factor.c (working copy)
> > > @@ -71,6 +71,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -104,6 +105,7 @@
> > >
> > > #endif
> > >
> > > +static bool is_hex(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(, buf) == 0 &&
> > > - BN_hex2bn(, buf) == 0)
> >
> > Why does this logic fail?
>
> See BN_hex2bn manpage. C/C++ does shortcircuits. With 1abc,
> BN_dec2bn converts the string to 1, puts it in val, and returns
> nonzero. BN_hex2bn is never called. Flipping the conditionals,
> of course, doesn't work because 0-9 are digits in the hexadecimal
> set (e.g., 111 is a valid hex and decimal string).
>
> > > - errx(1, "%s: illegal numeric format.", buf);
> > > + ch = is_hex(p) ? BN_hex2bn(, p) :
> > > + BN_dec2bn(, 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 == '-')
> > > errx(1, "negative numbers aren't permitted.");
> > > - if (BN_dec2bn(, argv[0]) == 0 &&
> > > - BN_hex2bn(, argv[0]) == 0)
> > > - errx(1, "%s: illegal numeric format.", argv[0]);
> > > + if (*p == '+') p++;
> > > + ch = is_hex(p) ? BN_hex2bn(, p) :
> > > + BN_dec2bn(, p);
> > > + if (ch == 0)
> > > + errx(1, "%s: illegal numeric format.", p);
> > > pr_fact(val);
> > > }
> > > exit(0);
> > > @@ -343,10 +349,9 @@
> > > BN_dec2bn(BIGNUM **a, const char *str)
> > > {
> > > char *p;
> > > -
> > This blank line is part of style(9)
> >
>
> Whoops. Haven't had to worry about style(9) in a long time.
>
> > > errno = 0;
> > > **a = strtoul(str, , 10);
> > > - return (errno == 0 && (*p == '\n' || *p == '\0'));
> > > + return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
> > > }
> > >
> > > static int
> > > @@ -356,7 +361,7 @@
> > >
> > > errno = 0;
> > > **a = strtoul(str, , 16);
> >
Re: OpenSSL breaks factor(6)
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:
> > > On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote:
> > > > The use of OpenSSL in factor(6) breaks factor(6) with respect to
> > > > its documentation.
> > > >
> > > > % man factor
> > > > ...
> > > > Numbers may be preceded by a single '+'.
> > > > ...
> > > >
> > > > % factor +125
> > > > factor: +125: illegal numeric format.
> > > >
> > >
> > > This fixes factor(6) for the above issue. The issue with
> > > hexadecimal is not easily fixed.
> > >
> >
> > 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.
> >
> > Index: factor.c
> > ===
> > --- factor.c(revision 355983)
> > +++ factor.c(working copy)
> > @@ -71,6 +71,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -104,6 +105,7 @@
> >
> > #endif
> >
> > +static boolis_hex(char *str);
> > static voidBN_print_dec_fp(FILE *, const BIGNUM *);
> >
> > static voidpr_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(, buf) == 0 &&
> > - BN_hex2bn(, buf) == 0)
>
> Why does this logic fail?
See BN_hex2bn manpage. C/C++ does shortcircuits. With 1abc,
BN_dec2bn converts the string to 1, puts it in val, and returns
nonzero. BN_hex2bn is never called. Flipping the conditionals,
of course, doesn't work because 0-9 are digits in the hexadecimal
set (e.g., 111 is a valid hex and decimal string).
> > - errx(1, "%s: illegal numeric format.", buf);
> > + ch = is_hex(p) ? BN_hex2bn(, p) :
> > + BN_dec2bn(, 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 == '-')
> > errx(1, "negative numbers aren't permitted.");
> > - if (BN_dec2bn(, argv[0]) == 0 &&
> > - BN_hex2bn(, argv[0]) == 0)
> > - errx(1, "%s: illegal numeric format.", argv[0]);
> > + if (*p == '+') p++;
> > + ch = is_hex(p) ? BN_hex2bn(, p) :
> > + BN_dec2bn(, p);
> > + if (ch == 0)
> > + errx(1, "%s: illegal numeric format.", p);
> > pr_fact(val);
> > }
> > exit(0);
> > @@ -343,10 +349,9 @@
> > BN_dec2bn(BIGNUM **a, const char *str)
> > {
> > char *p;
> > -
> This blank line is part of style(9)
>
Whoops. Haven't had to worry about style(9) in a long time.
> > errno = 0;
> > **a = strtoul(str, , 10);
> > - return (errno == 0 && (*p == '\n' || *p == '\0'));
> > + return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
> > }
> >
> > static int
> > @@ -356,7 +361,7 @@
> >
> > errno = 0;
> > **a = strtoul(str, , 16);
> > - return (errno == 0 && (*p == '\n' || *p == '\0'));
> > + return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
> > }
> >
> > static BN_ULONG
> > @@ -370,3 +375,17 @@
> > }
> >
> > #endif
> > +
> > +/* Check if the string contains a hexadecimal digit. */
> > +static bool
> > +is_hex(char *str)
> This function is poorly named as it does not check for
> all valid hex digits, only for alpha hex digits. It
> also only accepts lower case hex alpha, I would expect
> hex input to be case insensitive.
>
>
Re: OpenSSL breaks factor(6)
> On Fri, Dec 27, 2019 at 02:42:12PM -0800, Steve Kargl wrote: > > On Fri, Dec 27, 2019 at 01:47:17PM -0800, Steve Kargl wrote: > > > On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote: > > > > The use of OpenSSL in factor(6) breaks factor(6) with respect to > > > > its documentation. > > > > > > > > % man factor > > > > ... > > > > Numbers may be preceded by a single '+'. > > > > ... > > > > > > > > % factor +125 > > > > factor: +125: illegal numeric format. > > > > > > > > > > This fixes factor(6) for the above issue. The issue with > > > hexadecimal is not easily fixed. > > > > > > > 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. > > Might as well fix the documentation. Acknowledges that factor(6) > can deal with hexidecimal strings, and as a bonus fixes the bogus > information about the maximum value considered by primes(6). > > Index: factor.6 > === > --- factor.6 (revision 355983) > +++ factor.6 (working copy) > @@ -36,7 +36,7 @@ > .\" > .\" chongo /\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 a ^contain? s/a/an/ > +.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 > > -- > Steve -- Rod Grimes rgri...@freebsd.org ___ 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"
Re: OpenSSL breaks factor(6)
> On Fri, Dec 27, 2019 at 01:47:17PM -0800, Steve Kargl wrote:
> > On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote:
> > > The use of OpenSSL in factor(6) breaks factor(6) with respect to
> > > its documentation.
> > >
> > > % man factor
> > > ...
> > > Numbers may be preceded by a single '+'.
> > > ...
> > >
> > > % factor +125
> > > factor: +125: illegal numeric format.
> > >
> >
> > This fixes factor(6) for the above issue. The issue with
> > hexadecimal is not easily fixed.
> >
>
> 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?
>
> Index: factor.c
> ===
> --- factor.c (revision 355983)
> +++ factor.c (working copy)
> @@ -71,6 +71,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -104,6 +105,7 @@
>
> #endif
>
> +static bool is_hex(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(, buf) == 0 &&
> - BN_hex2bn(, buf) == 0)
Why does this logic fail?
> - errx(1, "%s: illegal numeric format.", buf);
> + ch = is_hex(p) ? BN_hex2bn(, p) :
> + BN_dec2bn(, 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 == '-')
> errx(1, "negative numbers aren't permitted.");
> - if (BN_dec2bn(, argv[0]) == 0 &&
> - BN_hex2bn(, argv[0]) == 0)
> - errx(1, "%s: illegal numeric format.", argv[0]);
> + if (*p == '+') p++;
> + ch = is_hex(p) ? BN_hex2bn(, p) :
> + BN_dec2bn(, p);
> + if (ch == 0)
> + errx(1, "%s: illegal numeric format.", p);
> pr_fact(val);
> }
> exit(0);
> @@ -343,10 +349,9 @@
> BN_dec2bn(BIGNUM **a, const char *str)
> {
> char *p;
> -
This blank line is part of style(9)
> errno = 0;
> **a = strtoul(str, , 10);
> - return (errno == 0 && (*p == '\n' || *p == '\0'));
> + return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
> }
>
> static int
> @@ -356,7 +361,7 @@
>
> errno = 0;
> **a = strtoul(str, , 16);
> - return (errno == 0 && (*p == '\n' || *p == '\0'));
> + return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
> }
>
> static BN_ULONG
> @@ -370,3 +375,17 @@
> }
>
> #endif
> +
> +/* Check if the string contains a hexadecimal digit. */
> +static bool
> +is_hex(char *str)
This function is poorly named as it does not check for
all valid hex digits, only for alpha hex digits. It
also only accepts lower case hex alpha, I would expect
hex input to be case insensitive.
is_hexalpha?
> +{
> + char c, *p;
> + for (p = str; *p; p++) {
> + c = tolower(*p);
> + if (c == 'a' || c == 'b' || c == 'c' || c == 'd' ||
> + c == 'e' || c == 'f')
if ( c >= 'a' || c <= 'f')
> + return true;
> + }
> + return false;
> +}
>
> --
> Steve
--
Rod Grimes rgri...@freebsd.org
___
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"
Re: OpenSSL breaks factor(6)
On Fri, Dec 27, 2019 at 02:42:12PM -0800, Steve Kargl wrote: > On Fri, Dec 27, 2019 at 01:47:17PM -0800, Steve Kargl wrote: > > On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote: > > > The use of OpenSSL in factor(6) breaks factor(6) with respect to > > > its documentation. > > > > > > % man factor > > > ... > > > Numbers may be preceded by a single '+'. > > > ... > > > > > > % factor +125 > > > factor: +125: illegal numeric format. > > > > > > > This fixes factor(6) for the above issue. The issue with > > hexadecimal is not easily fixed. > > > > 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. Might as well fix the documentation. Acknowledges that factor(6) can deal with hexidecimal strings, and as a bonus fixes the bogus information about the maximum value considered by primes(6). Index: factor.6 === --- factor.6(revision 355983) +++ factor.6(working copy) @@ -36,7 +36,7 @@ .\" .\" chongo /\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 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 -- 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"
Re: OpenSSL breaks factor(6)
On Fri, Dec 27, 2019 at 01:47:17PM -0800, Steve Kargl wrote:
> On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote:
> > The use of OpenSSL in factor(6) breaks factor(6) with respect to
> > its documentation.
> >
> > % man factor
> > ...
> > Numbers may be preceded by a single '+'.
> > ...
> >
> > % factor +125
> > factor: +125: illegal numeric format.
> >
>
> This fixes factor(6) for the above issue. The issue with
> hexadecimal is not easily fixed.
>
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.
Index: factor.c
===
--- factor.c(revision 355983)
+++ factor.c(working copy)
@@ -71,6 +71,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -104,6 +105,7 @@
#endif
+static boolis_hex(char *str);
static voidBN_print_dec_fp(FILE *, const BIGNUM *);
static voidpr_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(, buf) == 0 &&
- BN_hex2bn(, buf) == 0)
- errx(1, "%s: illegal numeric format.", buf);
+ ch = is_hex(p) ? BN_hex2bn(, p) :
+ BN_dec2bn(, 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 == '-')
errx(1, "negative numbers aren't permitted.");
- if (BN_dec2bn(, argv[0]) == 0 &&
- BN_hex2bn(, argv[0]) == 0)
- errx(1, "%s: illegal numeric format.", argv[0]);
+ if (*p == '+') p++;
+ ch = is_hex(p) ? BN_hex2bn(, p) :
+ BN_dec2bn(, p);
+ if (ch == 0)
+ errx(1, "%s: illegal numeric format.", p);
pr_fact(val);
}
exit(0);
@@ -343,10 +349,9 @@
BN_dec2bn(BIGNUM **a, const char *str)
{
char *p;
-
errno = 0;
**a = strtoul(str, , 10);
- return (errno == 0 && (*p == '\n' || *p == '\0'));
+ return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
}
static int
@@ -356,7 +361,7 @@
errno = 0;
**a = strtoul(str, , 16);
- return (errno == 0 && (*p == '\n' || *p == '\0'));
+ return (errno == 0 ? 1 : 0);/* OpenSSL returns 0 on error! */
}
static BN_ULONG
@@ -370,3 +375,17 @@
}
#endif
+
+/* Check if the string contains a hexadecimal digit. */
+static bool
+is_hex(char *str)
+{
+ char c, *p;
+ for (p = str; *p; p++) {
+ c = tolower(*p);
+ if (c == 'a' || c == 'b' || c == 'c' || c == 'd' ||
+ c == 'e' || c == 'f')
+ return true;
+ }
+ return false;
+}
--
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"
Re: OpenSSL breaks factor(6)
On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote:
> The use of OpenSSL in factor(6) breaks factor(6) with respect to
> its documentation.
>
> % man factor
> ...
> Numbers may be preceded by a single '+'.
> ...
>
> % factor +125
> factor: +125: illegal numeric format.
>
This fixes factor(6) for the above issue. The issue with
hexadecimal is not easily fixed.
Index: factor.c
===
--- factor.c(revision 355983)
+++ factor.c(working copy)
@@ -148,21 +148,23 @@
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(, buf) == 0 &&
- BN_hex2bn(, buf) == 0)
- errx(1, "%s: illegal numeric format.", buf);
+ if (BN_dec2bn(, p) == 0 &&
+ BN_hex2bn(, p) == 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 == '-')
errx(1, "negative numbers aren't permitted.");
- if (BN_dec2bn(, argv[0]) == 0 &&
- BN_hex2bn(, argv[0]) == 0)
- errx(1, "%s: illegal numeric format.", argv[0]);
+ if (*p == '+') p++;
+ if (BN_dec2bn(, p) == 0 &&
+ BN_hex2bn(, p) == 0)
+ errx(1, "%s: illegal numeric format.", p);
pr_fact(val);
}
exit(0);
--
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"
OpenSSL breaks factor(6)
The use of OpenSSL in factor(6) breaks factor(6) with respect to its documentation. % man factor ... Numbers may be preceded by a single '+'. ... % factor +125 factor: +125: illegal numeric format. without OpenSSL one gets % factor +125 125: 5 5 5 Although undocumented, factor(6) seems to support hexidecimal input, but does it? % factor abc 2748: 2 2 3 229 % factor 1abc 1: 1 without OpenSSL one gets % factor abc 2748: 2 2 3 229 % factor 1abc 6844: 2 2 29 59 The above behavior with OpenSSL appears to match the following documentation: % man factor ... Numbers are terminated by a non-digit character (such as a newline). except without OpenSSL, factor(6) produces % factor 1abc 6844: 2 2 29 59 where 6844 is the decimal representation of 0x1abc. But, one now finds % factor 1abcp factor: 1abcp: illegal numeric format. so, factor(6) without OpenSSL does not terminate numbers with non-digit characters. -- Steve 20161221 https://www.youtube.com/watch?v=IbCHE-hONow ___ 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"
