Bug 15993 - PCRE needs a bath
Summary: PCRE needs a bath
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-14 16:12 PST by Eric Seidel (no email)
Modified: 2007-11-14 17:17 PST (History)
1 user (show)

See Also:


Attachments
the patch (55.89 KB, patch)
2007-11-14 16:13 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Add a couple more minor cleanups to PCRE, -b diff (23.54 KB, patch)
2007-11-14 16:56 PST, Eric Seidel (no email)
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-11-14 16:12:48 PST
PCRE needs a bath

Wow.  Talk about hard to read code.

Anyway, in attempting to understand what jsRegExpCompile was doing, I ended up abstracting out the pattern length calculation and giving the entire method a bath.

All I did here was move out the length calculating code (along with associated local variables), move all variable declarations from the start of the block to where they are used (now that this is c++ and we can!), fix a few minor cases where the style disagreed with WebKit style (single line ifs, for example, where { goes, etc.)  I'm *certain* I did not catch all style violations. :)

There are *no* functional changes in this patch.  It passes all the tests.
Comment 1 Eric Seidel (no email) 2007-11-14 16:13:20 PST
Created attachment 17284 [details]
the patch
Comment 2 Eric Seidel (no email) 2007-11-14 16:17:46 PST
Oh, and the very very first thing I did was to ask Xcode to re-indent the entire jsRegExpCompile function so I had some clue where blocks started and ended. :)
Comment 3 Eric Seidel (no email) 2007-11-14 16:56:24 PST
Created attachment 17285 [details]
Add a couple more minor cleanups to PCRE, -b diff

This now includes some additional cleanup I'd been working on for my next patch.  However using -b to ignore whitespace makes for a smaller patch even with those extra changes.  I can get you the original diff ignoring whitespace if that would be easier.

My additional changes were to add a constructor to compile_data and to break out the debug printf code into its own separate function.  I also added returnError to replace the PCRE_ERROR_RETURN goto.

Yay for more-readable code!
Comment 4 Eric Seidel (no email) 2007-11-14 17:17:53 PST
r27802