Re: [RFC] Tokenizer API

On 12/09/2013 10:46 PM, Francesco Chemolli wrote:

> My suggestion is to have CharacterSet be a SBuf and
> rely on them, at least for now. In any case having them be a SBuf
> promotes better interface decoupling and abstraction.

CharacterSet is a "set of characters", which is semantically very
different from a "string". It would be overall wrong to abuse SBuf for
that purpose. It should either start simple (as I suggested) or
immediately optimized into a dedicated array-based class (as Amos proposed).

> Oh, one more argument for having the low-level matching primitives in
> SBuf: it's a pet peeve of mine to use some form of compact tries
> and/or FSM to do single-pass low-level string matching in SBuf

The purpose of Tokenizer is to help transform input into tokens using
simple rules. There is absolutely nothing in that design that prevents
someone from adding more matching methods to SBuf or optimizing existing
ones. Tokenizer can call those new/optimized methods as needed. That
does not lead to code duplication or even performance overheads.

On the other hand, it would be very wrong for the caller to use raw SBuf
methods to tokenize input. Tokenization is Tokenizer job, not SBuf job.
Please let me know if I should give more arguments explaining the
difference and supporting this design.

> SBuf was not really designed to be passed by nonconst reference.

I do not think that is a valid claim. Nearly any C++ object can be
passed by reference. There is nothing wrong with that as such, it is
often a good idea, and no general-purpose design can (or should)
prohibit that. The "const" part of the API (or lack of thereof) is just
an indication that the method argument cannot (or can) be modified.

>> const SBuf &remaining() const

> I'd change the signature to
> SBuf remaining() const

> copying a SBuf is easy, returning one puts a lower requirement on the
> caller and is less constrained

Copying SBuf is easy but relatively expensive compared to not copying. I
am not sure GCC would optimize that copy away when it can be. Here, I am
talking about calling various refcounting methods, not copying of data
buffers, of course.

The reference-based method would not allow the caller to modify the
return value directly, but can you think of a common use pattern or two
where such on-the-fly modification would actually be needed or desired?

Please note that we can change this aspect of the interface later if
needed without any need to change any of the callers (AFAICT).

> I'd also add to the interface a few constants to describe common
> character sets such as ALPHA, ALNUM, LOWERALPHA, UPPERALPHA etc. (I'd
> use the predefined character classes from grep(1) as a refetence for
> common patterns).

Yes, the proposed design will work well only if we predefine virtually
all character sets. There is no need for a new interface for that
though. Just declare global constant CharacterSet objects (or functions
creating/returning them).

However, I recommend against adding a lot of such constants until they
are needed by a specific caller. Why waste 256 bytes? It is easy to add
more as needed.

I would recommend adding a macro to create a CharacterSet from one of
the ISALPHA(3) isfoo() functions/macros, but I am worried that those are
locale-dependent while almost everything in Squid should not be. We may
be better off hard-coding any protocol-defined character sets instead.

Thank you,

Alex.
Received on Tue Dec 10 2013 - 15:37:24 MST