Discussion:
[PATCH] af_alg: Improve comments.
Paul Eggert
2018-05-06 01:47:43 UTC
Permalink
* lib/af_alg.h: Use imperatives and tighten up wording.
---
ChangeLog | 5 +++++
lib/af_alg.h | 21 +++++++++------------
2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8230d8470..7f1f5b032 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2018-05-05 Paul Eggert <***@cs.ucla.edu>
+
+ af_alg: Improve comments.
+ * lib/af_alg.h: Use imperatives and tighten up wording.
+
2018-05-05 Bruno Haible <***@clisp.org>

af_alg: Improve comments.
diff --git a/lib/af_alg.h b/lib/af_alg.h
index 295f0f9b5..2545ec6ec 100644
--- a/lib/af_alg.h
+++ b/lib/af_alg.h
@@ -1,5 +1,4 @@
-/* af_alg.h - Declaration of functions to compute message digest from
- file streams using Linux kernel crypto API.
+/* af_alg.h - Compute message digests from file streams.
Copyright (C) 2018 Free Software Foundation, Inc.

This program is free software; you can redistribute it and/or modify it
@@ -18,7 +17,7 @@
/* Written by Matteo Croce <***@redhat.com>, 2018.
Documentation by Bruno Haible <***@clisp.org>, 2018. */

-/* This file declares specific functions for computing message digests
+/* Declare specific functions for computing message digests
using the Linux kernel crypto API, if available. This kernel API gives
access to specialized crypto instructions (that would also be available
in user space) or to crypto devices (not directly available in user space).
@@ -38,13 +37,11 @@ extern "C" {

# if HAVE_LINUX_IF_ALG_H

-/* Computes a message digest of the contents of a file.
- STREAM is an open file stream. Regular files are handled more efficiently
- than other types of files.
- ALG is the message digest algorithm. The supported algorithms are listed in
- the file /proc/crypto.
- RESBLOCK points to a block of HASHLEN bytes, for the result. HASHLEN must be
- the length of the message digest, in bytes, in particular:
+/* Compute a message digest of the contents of a file.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ ALG is the message digest algorithm; see the file /proc/crypto.
+ RESBLOCK points to a block of HASHLEN bytes, for the result.
+ HASHLEN must be the length of the message digest, in bytes, in particular:

alg | hashlen
-------+--------
@@ -55,8 +52,8 @@ extern "C" {
sha384 | 48
sha512 | 64

- If successful, this function fills RESBLOCK and returns 0.
- Upon failure, it returns a negated error code. */
+ If successful, fill RESBLOCK and return 0.
+ Upon failure, return a negated error number. */
int
afalg_stream (FILE *stream, const char *alg, void *resblock, ssize_t hashlen);
--
2.17.0
Bruno Haible
2018-05-06 09:44:31 UTC
Permalink
This post might be inappropriate. Click to display it.
Paul Eggert
2018-05-06 10:11:38 UTC
Permalink
Post by Bruno Haible
The GNU Coding Standards [1] don't favour either, although I have vague
memories that 20 years ago, it advocated imperative style.
Yes, I have the same vague memory, and still prefer imperative style in many
cases. For routine use in comments, the imperative style is typically shorter
and easier to understand. It's easier to read "Declare specific functions for X"
than "This file declares specific functions for X", for example, and the longer
wording does not convey enough extra information to justify its extra length.

For specs that must contain qualifiers like "shall" or "should", the imperative
style does not work well, and that's a good reason for POSIX and other
more-abstract specifications to not use it.

One other beef I often have with comments (including some that I write!) is that
they are not sentences. At least this guideline *is* in the GNU Coding Standards.
Jim Meyering
2018-05-06 14:33:07 UTC
Permalink
Post by Paul Eggert
Post by Bruno Haible
The GNU Coding Standards [1] don't favour either, although I have vague
memories that 20 years ago, it advocated imperative style.
Yes, I have the same vague memory, and still prefer imperative style in many
cases. For routine use in comments, the imperative style is typically
shorter and easier to understand. It's easier to read "Declare specific
functions for X" than "This file declares specific functions for X", for
example, and the longer wording does not convey enough extra information to
justify its extra length.
For specs that must contain qualifiers like "shall" or "should", the
imperative style does not work well, and that's a good reason for POSIX and
other more-abstract specifications to not use it.
One other beef I often have with comments (including some that I write!) is
that they are not sentences. At least this guideline *is* in the GNU Coding
Standards.
Here's the relevant section: (from
https://www.gnu.org/prep/standards/html_node/GNU-Manuals.html)

Whenever possible, please stick to the active voice, avoiding the
passive, and use the present tense, not the future teste. For
instance, write “The function foo returns a list containing a and b”
rather than “A list containing a and b will be returned.” One
advantage of the active voice is it requires you to state the subject
of the sentence; with the passive voice, you might omit the subject,
which leads to vagueness.
John W. Eaton
2018-05-07 13:46:25 UTC
Permalink
Post by Jim Meyering
Here's the relevant section: (from
https://www.gnu.org/prep/standards/html_node/GNU-Manuals.html)
Whenever possible, please stick to the active voice, avoiding the
passive, and use the present tense, not the future teste. For
instance, write “The function foo returns a list containing a and b”
rather than “A list containing a and b will be returned.” One
advantage of the active voice is it requires you to state the subject
of the sentence; with the passive voice, you might omit the subject,
which leads to vagueness.
Has it always been that way? I also have the vague recollection of RMS
telling people to write

Return a list containing a and b.

instead of

The function foo returns ...

or even

Returns a list ...

This imperative style is used in Emacs doc strings, which is probably
why I came to prefer it.

I agree it is somewhat of a personal preference, but probably worth
being consistent within a project, whatever style is selected.

jwe
Bruno Haible
2018-05-07 14:57:39 UTC
Permalink
I also have the vague recollection of RMS telling people to write
Return a list containing a and b.
instead of
The function foo returns ...
or even
Returns a list ...
He apparently doesn't do this any more. Recently RMS reviewed the
documentation of GNU libunistring [1]. He suggested to avoid passive voice.
But although this documentation uses the descriptive style ("Returns the ...")
consistently, he did not suggest to change that.

Bruno

[1] https://www.gnu.org/software/libunistring/manual/libunistring.html
Loading...