Pádraig Brady wrote:
> On 10/23/2012 08:46 AM, Jim Meyering wrote:
>> Torbjorn Granlund wrote:
>>> Pádraig Brady <p...@draigbrady.com> writes:
...
>>> Aren't this Draconian options ("ask the compiler to nag about everything
>>> it can, then treat nagging as errors in the code") self-defeating? You
>>> need to avoid natural ways of writing things, and instead go for
>>> contorted solutions. I spent a few hours trying to satisfy your demands
>>> in this area, and I am sure you maintainers have spent a lot of time on
>>> it.
>>
>> We've learned that while many of the warnings are
>> not about real bugs, some do indicate real problems.
>> An unused macro or variable could well indicate a typo
>> in the definition/declaration or in all uses.
>>
>>> I for one prefer to spend my time on improving GNU, not define an
>>> artifical goal like this one, and work on satifying that.
>>
>> We have moved to using very strict warning settings gradually over the
>> years, carefully disabling the few warning options for which the cost
>> consistently outweighs the benefit.
>>
>> We really do believe that what we are doing is worthwhile.
>
> The onus is on those who know the code (now),
> to minimize warnings to ease maintenance down the line.
Precisely. We are in the unusual position of being able to take
a very long view of software "health". That puts a large weight
on long-term maintainability, sharing responsibility, documenting, etc.
In that spirit, and considering the little pang :-) I felt last night at
the dearth of comments in factor.c, we should add more comments there.
Each function should have a comment that describes what it does
in terms of its arguments. To see a list of candidates, run this
grep -A1 -B3 '^static' src/factor.c
Many of its macros deserve comments, too.
I'll start:
(just writing the comment for factor made me see
that I would prefer to have it take an algorithm-specifying
parameter rather than use that global variable.)
>From 225f340c367e2618b3e7d56c506893ea1c6e567f Mon Sep 17 00:00:00 2001
From: Jim Meyering <j...@meyering.net>
Date: Tue, 23 Oct 2012 12:08:58 +0200
Subject: [PATCH] factor: add comments
* src/factor.c (is_square): Use active voice in comment, not passive.
(factor): Add function-describing comment.
(mp_factor): Likewise.
---
src/factor.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/factor.c b/src/factor.c
index 73c59e9..e7d152b 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -1792,7 +1792,7 @@ isqrt2 (uintmax_t nh, uintmax_t nl)
#define MAGIC65 ((uint64_t) 0x218a019866014613ULL)
#define MAGIC11 0x23b
-/* Returns the square root if the input is a square, otherwise 0. */
+/* Return the square root if the input is a square, otherwise 0. */
static uintmax_t _GL_ATTRIBUTE_CONST
is_square (uintmax_t x)
{
@@ -2167,6 +2167,8 @@ factor_using_squfof (uintmax_t n1, uintmax_t n0, struct
factors *factors)
return false;
}
+/* Compute the prime factors of the 128-bit number (T1,T0), and put the
+ results in FACTORS. Use the algorithm selected by the global ALG. */
static void
factor (uintmax_t t1, uintmax_t t0, struct factors *factors)
{
@@ -2197,6 +2199,8 @@ factor (uintmax_t t1, uintmax_t t0, struct factors
*factors)
}
#if HAVE_GMP
+/* Use Pollard-rho to compute the prime factors of
+ arbitrary-precision T, and put the results in FACTORS. */
static void
mp_factor (mpz_t t, struct mp_factors *factors)
{
--
1.8.0
