1227624 - NSS clang-format: lib/certcb

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Franziskus Kiefer [:franziskus]]

Applying clang-format to lib/certdb.

Comment 1

[Franziskus Kiefer [:franziskus]]

Attachment: lib/certdb clang-format protection

Comment 2

[Franziskus Kiefer [:franziskus]]

Attachment: lib/certdb clang-format

here another patch with clang format and protection (first patch) for lib/certdb. Who ever has time for this can get to review it, otherwise just cancel the needinfo.

For better context, both patches are also uploaded here
protection: https://codereview.appspot.com/275560043/
clang-format: https://codereview.appspot.com/275560044/

Comment 3

[Franziskus Kiefer [:franziskus]]

I updated both patches

Comment 4

[Franziskus Kiefer [:franziskus]]

Attachment: lib/certdb clang-format protection

r+ by wtc on Rietval (https://codereview.appspot.com/275560043/)

Comment 5

[Wan-Teh Chang]

Comment on attachment 8694192 [details] [diff] [review]
lib/certdb clang-format protection

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

r=wtc. Please fix the two nits I noted.

::: lib/certdb/alg1485.c
@@ +27,4 @@
>  /* Add new entries to this table, and maybe to function ParseRFC1485AVA */
>  static const NameToKind name2kinds[] = {
>  /* IANA registered type names
>   * (See: http://www.iana.org/assignments/ldap-parameters) 

Nit: please delete the space at the end of this line
(visible in the code review tool).

::: lib/certdb/genname.c
@@ +1585,1 @@
>  #define ANSSI_SUBJECT_DN \

Nit: I think this backslash should be moved to the end of the line
to match clang-format?

Comment 6

[Franziskus Kiefer [:franziskus]]

Attachment: lib/certdb clang-format protection

fixed nits, r+ carries over

Comment 7

[Kai Engert (:KaiE:)]

Comment on attachment 8694651 [details] [diff] [review]
lib/certdb clang-format protection

https://hg.mozilla.org/projects/nss/rev/0b33a0841798

Comment 8

[Franziskus Kiefer [:franziskus]]

Attachment: lib/certdb clang-format

rebased after protection changes

Comment 9

[Kai Engert (:KaiE:)]

In file lib/certdb/alg1485.c, in cert_AVAOidTagToMaxLen and the functions that follow after it, it uses an indent of 2 spaces, not 4.

I wonder why that happened, because in other files it uses the correct indent of 4 spaces.

I see our format definition has ObjCBlockIndentWidth: 2

Does clang-format incorrectly assume that file alg1485.c is an object-c file ?

Does it use 4 spaces if you set ObjCBlockIndentWidth to 4 ?

Comment 10

[Kai Engert (:KaiE:)]

FWIW, (unpatched) clang-format version 3.7 correctly indents alg1485.c with 4 spaces.

Comment 11

[Franziskus Kiefer [:franziskus]]

Attachment: lib/certdb clang-format

thanks kai. not sure what happened to that file. This is a new patch where the indent looks correct (it's the same as the last one at https://codereview.appspot.com/275560044 if you want to check the differences).

Comment 12

[Kai Engert (:KaiE:)]

Comment on attachment 8696937 [details] [diff] [review]
lib/certdb clang-format

r=kaie

I skimmed through it, and it seems OK.

I found the following minor issues, but I think, if we want to fix them, they should be done in a follow-up patch.


(a)
+#define CASE(i, m)  \
+    case i:         \
+        CGET(i, m); \
+        if (!n)     \
+        goto unsupported /* fall-through */

indent the goto


(b)
+        /* If tag name is too long, we know it is an OID form that was
+     * allocated from the heap, so we can modify it in place
+     */

The first line of a multi line comment gets indented incorrectly.
There are multiple places like this.


(c)
     if (rv == SECSuccess &&
-        ((trust.sslFlags & CERTDB_USER ) ||
-         (trust.emailFlags & CERTDB_USER ) ||
-         (trust.objectSigningFlags & CERTDB_USER )) ) {
+        ((trust.sslFlags & CERTDB_USER) || (trust.emailFlags & CERTDB_USER) ||
+         (trust.objectSigningFlags & CERTDB_USER))) {


The original style was more readable, but I think the new one is acceptable and can be kept.

Comment 13

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/projects/nss/rev/3896627e2a2f