1251185 - NSS clang-format: else line-break fixes

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Franziskus Kiefer [:franziskus]]

Everything clang-format patch that landed until now was produced with a clang-format version containing a bug and introducing a line break before else statements.

Instead of 
> ...
> } else {
> ...

we have
> ...
> }
> else {
> ...

in formatted files.
This bug his since been fixed (http://reviews.llvm.org/D15485). In this bug we should fix those cases.

Comment 1

[Franziskus Kiefer [:franziskus]]

Attachment: else-fix.patch

the patch is also at https://codereview.appspot.com/282660043

this should fix the else cuddling and some other minor things clang-format and I were missing before.

Kai can you have a look? we should make sure that no one has a big patch that would conflict.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8723492 [details] [diff] [review]
else-fix.patch

Review of attachment 8723492 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. I looked at every change and they're fine, assuming that the binary diff agrees. There are a few unrelated changes in there but they're fine too, except for what I noted. I expect this to bitrot probably some patches (mine and mt's at least) but I think the impact here will be less than the libssl reformatting.

::: lib/certhigh/certvfy.c
@@ +1027,5 @@
>                  flags = trust.sslFlags;
>  
>                  /* is the cert directly trusted or not trusted ? */
>                  if (flags & CERTDB_TERMINAL_RECORD) { /* the trust record is
> +                            * authoritative */

This seems off.

@@ +1118,5 @@
>              case certUsageUserCertImport:
>                  /* do we distrust these certs explicitly */
>                  flags = trust.sslFlags;
>                  if (flags & CERTDB_TERMINAL_RECORD) { /* the trust record is
> +                            * authoritative */

Off too.

@@ +1126,5 @@
>                      }
>                  }
>                  flags = trust.emailFlags;
>                  if (flags & CERTDB_TERMINAL_RECORD) { /* the trust record is
> +                            * authoritative */

And here.

::: lib/dbm/src/dirent.h
@@ +60,5 @@
>      unsigned short d_date;      /* modification date */
>  #else
>      char d_name[MAXNAMLEN + 1]; /* garentee null termination */
>      char d_attribute;           /* .. extension .. */
> +    unsigned long d_size; /* .. extension .. */

This looked better before.

::: lib/dev/devutil.c
@@ +172,5 @@
>  
>  /* object cache for token */
>  
>  typedef struct
> +{

It's weird how inconsistent this is with the formatting in for example sslmutex.h.

::: lib/ssl/ssl3con.c
@@ +2852,5 @@
>                                  wrBuf->buf + headerLen + ivLen, /* output */
>                                  &cipherBytesPart1,              /* actual outlen */
>                                  p1Len,                          /* max outlen */
>                                  pIn,
> +                                p1Len); /* input, and inputlen */

Looked better before.

Comment 3

[Kai Engert [:KaiE:]]

FWIW, I ignored such changes during the review of the bigger patches.

Comment 4

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/projects/nss/rev/e1aec44f44cc

Comment 5

[Kai Engert [:KaiE:]]

(In reply to Kai Engert (:kaie) from comment #4)
> https://hg.mozilla.org/projects/nss/rev/e1aec44f44cc

wrong link.

correct commit was: https://hg.mozilla.org/projects/nss/rev/0e6e8153513e