bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: factor


From: ThMO
Subject: Re: factor
Date: Mon, 03 Oct 2005 18:42:08 +0200

Hello Jim and others listening,

> > But your patch introduces a bug - it's possible now to give an invalid
> > number on the command line, which would normally be rejected by strtol():
> > -  ./gfactor -- '  -  130'
> > -  ./gfactor -- '  -  +131'
> > xstrtoumax() now skips the white spaces between `-' and the number itself,
> > which is wrong in this case; so if a minus sign has been scanned, you need
> > to ensure, that there is a digit following immediately.  The same applies
> > for a possible `+' sign.
> 
> Good catch.
> That's easy to fix.
> Change this:
>   is_negative = (*p == '-');
> to this:
>   is_negative = (*p == '-' && ISDIGIT (p[1]));

yep, that looks better.

> -       error (0, 0, _("%s is not a valid positive integer"), quote (s));
> +       error (0, 0, _("invalid argument: %s"), quote (s_diag));

Why not call this error message:  _("%s is not a valid number")  ?
That way the user immediately knows, that a number is needed.

> > That's the reason, why I've preferred to let this dirty work handed over
> > to the conversion routines, as otherwise I'll have to reinvent the wheel
> > over and over again...
> 
> But your patch limited the maximum magnitude to that of intmax_t,
> which is half that of the current limit.

You're right here.

> Thanks for the quick review.

No problem - we do have a holiday today...  ;-)

---

> If you're really interested in improving this program, extending it to
> use GMP[*] (i.e., -lgmp) is the way to go.  Then, it will be able to
> accept arbitrarily long strings of digits, and of course it'll be
> much more efficient for large numbers.

While looking at the sources, I found already a `factorize.c' program under
the `demos' directory - at least in the archive of the locally installed
v3.1.1 of the libgmp source tree.
This library is able to factorize the two examples in the coreutils info-page,
which are very slow, relatively fast.  After requesting another number, I had
to stop the execution after 12+ minutes - cross-checking it with pari, it took
only 191 seconds to factorize the following number:
  '82710182818190128129228267632297310019018273821100101836627627'
My still preferred approach in writing a wrapper-script around `gp' will result
in faster execution - although it might be a speed-up for gfactor to use the
gmp library.
Surely it must be investigated, how fast gfactor's algorithm will run using
libgmp...
If I find the mood tommorow, I'll eventually write a small hack using libgmp,
but I can't promise you that...

Finally I wish you all a good afternoon for now.

THX for listening.

CU Tom.
(Thomas M.Ott)
Germany




reply via email to

[Prev in Thread] Current Thread [Next in Thread]