Tidy Allocators Patch

This patch should applies cleanly against the September 2003 version of tidy. It comprises a number of earlier patches that correct memory leaks, several bugs that can cause access violations, and work to add support for per-document allocators.

Any comments or suggestions would be appreciated, send to mark@npsl.co.uk. See more free stuff by mark here.

Download

Initial version:
allocators.zip (24k)

Commentary

Stuff from the mail posted to the tidy-dev list with commentary:

Here's the gist:

typedef struct _TidyAllocatorVtbl {
    void* (*alloc)( TidyAllocator *self, size_t nBytes );
    void* (*realloc)( TidyAllocator *self, void *block, size_t nBytes );
    void (*free)( TidyAllocator *self, void *block);
    void (*panic)( TidyAllocator *self, ctmbstr msg );
} TidyAllocatorVtbl;

typedef struct _TidyAllocator {
    const TidyAllocatorVtbl *vtbl;
} TidyAllocator;

To declare a custom allocator, you do:

typedef struct _MyAllocator {
    TidyAllocator base;
    /* other state */
} MyAllocator;

then:

MyAllocator.base.vtbl = &myVtbl;

(declaring myVtbl somewhere). The allocator always gets back a pointer to itself, which it can cast to the correct type (MyAllocator above) and then access state as appropriate.

New version of existing functions are available:

tidyCreateWithAllocator
tidyBufInitWithAllocator
tidyAllocBufWithAllocator

The allocators for user provided buffers and documents can be different (AFAICT, I haven't tested this assertion in detail).

Other things to note are:

• This has been pretty well tested -- against the tidy test suite and then around 30Gb of randomly collected HTML (yes, that takes a while :)
• Includes the original clean up patch. This is mainly because it builds on this work; passing a number of extra function parameters around where necessary (e.g. to FreeNode) to ensure that all memory used by tidy is actually freed. Patches should apply against the current CVS cleanly.
• Should be backwardly compatible. Internally, MemAlloc and friends are replaced by a default allocator, which is basically the same as the old one. This still supports calling the old style memory allocation hooks where necessary. The default allocator is used if NULL is passed in, or if an old style tidyCreate() or tidyBufInit() is used.
• All allocations are now peformed via the macros TidyAlloc, TidyRealloc, TidyFree. These are expanded inline, and just call the appropriate function on the given allocator. ClearMemory has also been redefined as TidyClearMemory and is a macro that expands to memset. This is useful because memset inlines on most(?) compilers with lots of optimisation, and wrapping it in a function nukes that gain.
• Corresponding allocation macros exist for the document: TidyDocAlloc, etc. These use the document allocator (just saves a bit of typing)

There are an extra couple of fixes from the original cleanup:

• lexer.c#694

  void AddByte( Lexer *lexer, tmbchar ch )
  {
      if ( lexer->lexsize + 1 >= lexer->lexlength ) {
          ...
          while ( lexer->lexsize + 1 >= allocAmt )
      }
      lexer->lexbuf[ lexer->lexsize++ ] = ch;
      lexer->lexbuf[ lexer->lexsize ]   = '\0';  /* debug */
  

  The +1 here is wrong, and can overwrite the end of buffer. This only happens under rather wacky circumstances because the buffer starts at 8k and doubles. Changing it to +2 is OK.
• lexer.c#2547

  Things that look like XML processing instructions can cause tidy to run off the end of the lexer buffer. This happens:

  
      for (i = 0; i <= lexer->lexsize &&
           !IsWhite(lexer->lexbuf[i + lexer->txtstart]); ++i)
          /**/;
  

  Since txtstart is somewhere in the lexer buffer. Changing this to:

      for (i = 0; i > lexer->lexsize - lexer->txtstart &&
           !IsWhite(lexer->lexbuf[i + lexer->txtstart]); ++i)
         /**/;
  

  corrects the problem. It's not usually seen, I guess it depends what is off the end of the buffer.

The final issue is what to with file handles, and such like. This is not as such something that directly affects the patch; more that the only way to carry going after a malloc failure is to use setjmp/longjmp to get out of it. This potentially leaves open files (and their associated structures), which is a pretty fatal kind of leak (you tend to run out of file handles faster than memory). You can get around this by managing all of the files for tidy (passing them as custom sinks/sources), but it would be nice not to have to. The only clean way I can see around this is to decorate nearly _everything_ that allocates memory, and have it potentially return a failure. Then at the top level, you can return ENOMEM from the API call.