[webkit-reviews] review denied: [Bug 16036] Further cleanup to PCRE : [Attachment 17349] [PATCH] Reformat find_fixedlength

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Nov 18 19:32:13 PST 2007


Sam Weinig <sam at webkit.org> has denied 's request for review:
Bug 16036: Further cleanup to PCRE
http://bugs.webkit.org/show_bug.cgi?id=16036

Attachment 17349: [PATCH] Reformat find_fixedlength
http://bugs.webkit.org/attachment.cgi?id=17349&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
The * should go next to the type here.
+static int find_fixedlength(uschar *code, int options)

and here
+    uschar *cc = code + 1 + LINK_SIZE;

I think should be while (true) for consistency.
+    for (;;) {

Brace should go on the same line as the switch statement.
+	 switch (op)
+	 {

I think spliting this onto multiple lines will make it more clear. (appears
twice)
+		 do cc += GET(cc, 1); while (*cc == OP_ALT);

In the inner switch statement you indent the cases differently than the outer
switch.  Consistency is the key.
+		 switch (*cc) {
+		 case OP_CRSTAR:

r-


More information about the webkit-reviews mailing list