Pádraig Brady wrote:
On 10/23/2012 08:46 AM, Jim Meyering wrote:
Torbjorn Granlund wrote:
Pádraig Brady <address@hidden> 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 <address@hidden>
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