Mailing List Archive: [PATCH 4/4] whitespace changes

[PATCH 4/4] whitespace changes

Dec 22, 2021, 12:23 AM

Post #1 of 5 (523 views)

Break (most) long lines.
---
ssh-keygen.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 3a57925e..1f79f27c 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -2515,7 +2515,8 @@ load_sign_key(const char *keypath, const struct sshkey *pubkey)

static int
sign_one(struct sshkey *signkey, const char *filename, int fd,
- const char *sig_namespace, const char *hashalg, sshsig_signer *signer, void *signer_ctx)
+ const char *sig_namespace, const char *hashalg, sshsig_signer *signer,
+ void *signer_ctx)
{
struct sshbuf *sigbuf = NULL, *abuf = NULL;
int r = SSH_ERR_INTERNAL_ERROR, wfd = -1, oerrno;
@@ -2606,8 +2607,8 @@ sign_one(struct sshkey *signkey, const char *filename, int fd,
}

static int
-sig_process_opts(char * const *opts, size_t nopts, char *hashalg, size_t hashalg_size, uint64_t *verify_timep,
- int *print_pubkey)
+sig_process_opts(char * const *opts, size_t nopts, char *hashalg,
+ size_t hashalg_size, uint64_t *verify_timep, int *print_pubkey)
{
size_t i;
time_t now;
@@ -2648,7 +2649,8 @@ sig_process_opts(char * const *opts, size_t nopts, char *hashalg, size_t hashalg
}

static int
-sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv, char * const *opts, size_t nopts)
+sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv,
+ char * const *opts, size_t nopts)
{
int i, fd = -1, r, ret = -1;
int agent_fd = -1;
--
2.30.2

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH 4/4] whitespace changes [ In reply to ]

openssh at davidnewall

Dec 22, 2021, 10:08 PM

Post #2 of 5 (522 views)

Permalink

On 22/12/21 7:23 pm, Linus Nordberg wrote:
> Break (most) long lines.
I don't like this because it adds noise when comparing changes, and it's
rooted in very old fashioned thinking wherein lines must be wrapped at
72 characters. We no longer use punched cards, nor 80x24 terminals;
let's not keep doing this.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH 4/4] whitespace changes [ In reply to ]

djm at mindrot

Dec 23, 2021, 8:02 PM

Post #3 of 5 (522 views)

Permalink

On Thu, 23 Dec 2021, David Newall wrote:

> On 22/12/21 7:23 pm, Linus Nordberg wrote:
> > Break (most) long lines.
> I don't like this because it adds noise when comparing changes, and it's
> rooted in very old fashioned thinking wherein lines must be wrapped at
> 72 characters. We no longer use punched cards, nor 80x24 terminals;
> let's not keep doing this.

No, sorry. OpenSSH follows the OpenBSD style guide, which recommends 80
column wrapping. https://man.openbsd.org/style.9

-d
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH 4/4] whitespace changes [ In reply to ]

openssh at davidnewall

Dec 23, 2021, 9:32 PM

Post #4 of 5 (522 views)

Permalink

On 24/12/21 3:02 pm, Damien Miller wrote:
> No, sorry. OpenSSH follows the OpenBSD style guide, which recommends 80
> column wrapping.https://man.openbsd.org/style.9

The patch wraps at well longer than 80 characters.

The current source code is even longer.

From style.9: "These guidelines should be followed for all new code."
It's not new code.

Why change it? It helps not one whit but it does hurt.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH 4/4] whitespace changes [ In reply to ]

jmknoble at pobox

Dec 24, 2021, 1:41 PM

Post #5 of 5 (522 views)

Permalink

> On Dec 23, 2021, at 21:36, David Newall <openssh@davidnewall.com> wrote:
>
> From style.9: "These guidelines should be followed for all new code." It's not new code.
>
> Why change it? It helps not one whit but it does hurt.

Best practice for line-length, whitespace, and other style changes is:

(1) When changing a source file with legacy style, update the file to confirm to current style guidelines. This keeps tech debt from metastasizing.

(2) When making style changes, *put them in a separate change*. Combining style changes and semantic changes means that, as has been pointed out, changes are difficult to review in order to find the actual semantic change. Unit tests can also be run to detect any errors or regression the style change might include.

(2.1) Ideally, style changes should be made by automated tooling rather than manually. (Ideally ideally, the configuration/invocation for the automated tooling should be included in the source repo along with the code).

(3) Generally, make the style change first, then the semantic change. This enables tech debt reduction to happen even if the semantic change is rejected or delayed.

--
jmk

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev