In general the formatting is good. I found some problems. 1. Some lines are too long. 2. Some multi-line comment blocks are indented incorrectly. 3. Some line-continuation backslashes were added incorrectly. 4. Some conditional expressions were not formatted well. A short line is added even though the previous line still has room at the end. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certhigh.c File lib/certhigh/certhigh.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certhigh.c#newco... lib/certhigh/certhigh.c:209: return (certList); Ideally, statements like return(x); should be changed to return x; We may need to fix these manually. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certhigh.c#newco... lib/certhigh/certhigh.c:379: (trust->emailFlags & (CERTDB_VALID_CA | CERTDB_VALID_PEER)) || Could you please check if these lines are longer than 80 characters? https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certreq.c File lib/certhigh/certreq.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certreq.c#newcod... lib/certhigh/certreq.c:250: void (*setExts)(void *object, CERTCertExtension **exts)); Is this line longer than 80 characters? https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c File lib/certhigh/certvfy.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:293: goto loser; \ IMPORTANT: these line-continuation backslashes look wrong. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:336: SECSuccess) { Nit: it is strange why this isn't on the previous line. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:366: */ IMPORTANT: this comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:377: */ IMPORTANT: this comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:655: 0; IMPORTANT: This should be on the previous line. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:935: * authoritative */ This comment block is indented incorretly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:949: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:960: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:972: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:987: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:1026: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:1034: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:1044: * authoritative */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfy.c#newcod... lib/certhigh/certvfy.c:1071: void *wincx, CERTVerifyLog *log, SECCertificateUsage *returnedUsages) Is this line longer than 80 characters? https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfypkix.c File lib/certhigh/certvfypkix.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfypkix.c#ne... lib/certhigh/certvfypkix.c:1139: : PKIX_TRUE; Nit: this looks ugly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfypkix.c#ne... lib/certhigh/certvfypkix.c:1636: (PRBool)(param->value.scalar.b != This line looks too long! https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certvfypkix.c#ne... lib/certhigh/certvfypkix.c:2142: * Jumping to cleanup. */ This comment block is indented incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocsp.c File lib/certhigh/ocsp.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocsp.c#newcode700 lib/certhigh/ocsp.c:700: OCSP_Global.maximumSecondsToNextFetchAttempt * This line seems too long. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocsp.c#newcode706 lib/certhigh/ocsp.c:706: OCSP_Global.minimumSecondsToNextFetchAttempt * This line seems too long. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocsp.c#newcode1812 lib/certhigh/ocsp.c:1812: goto loser; \ These line-continuation backslashes look wrong. I think it's because line 1811 is too long. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocsp.c#newcode3819 lib/certhigh/ocsp.c:3819: * changed. (Hopefully it will!) \ IMPORTANT: line-continuation backslashes were added incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocsp.c#newcode4521 lib/certhigh/ocsp.c:4521: #define OCSP_SLOP (5L * 60L) /* OCSP responses are allowed to be 5 minutes \ A line-continuation backslash was added incorrectly. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocspsig.c File lib/certhigh/ocspsig.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocspsig.c#newcode75 lib/certhigh/ocspsig.c:75: SECSuccess) This should be on the previous line. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/ocspsig.c#newcod... lib/certhigh/ocspsig.c:195: } Nit: the last array element is not formatted well. Compare it with lines 224 and 245. I think we can work around this by removing the comma (,) after 0. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/xcrldist.c File lib/certhigh/xcrldist.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/xcrldist.c#newco... lib/certhigh/xcrldist.c:89: point->derFullName = cert_EncodeGeneralNames(ourPool, point->distPoint.fullName); This line is too long. https://codereview.appspot.com/218010043/diff/1/lib/certhigh/xcrldist.c#newco... lib/certhigh/xcrldist.c:173: DistributionPointNameTemplate, &(point->derDistPoint)); This line is too long.
https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certhigh.c File lib/certhigh/certhigh.c (right): https://codereview.appspot.com/218010043/diff/1/lib/certhigh/certhigh.c#newco... lib/certhigh/certhigh.c:379: (trust->emailFlags & (CERTDB_VALID_CA | CERTDB_VALID_PEER)) || On 2015/04/17 22:26:46, wtc1 wrote: > Could you please check if these lines are longer than 80 characters? This seems to be due to case labels being indented. Could we fix that by having case labels flush with the outer switch statement? e.g., switch(x) { case y: doY(); break; default: }