[squid-dev] [PATCH] suffix parsing and skipping

[Alex Rousskov]

On 07/23/2015 01:46 AM, Kinkie wrote:

> the only thing I don't understand is the XXX in
> parsedSize(); it's not clear why the comment, 

The parsedSize() method is broken: It claims to return the number of
bytes parsed, but it does not return that. It usually returns the number
of bytes parsed from the left, but not from the right of the string. For
example,

  skipOne()
  skipOneTrailing()

should usually result in parsedSize() returning 2 because 2 bytes were
parsed and removed from the string. It usually returns 1.


> and what to use instead.

There is currently no good alternative. Discussing the best way forward
is outside this project scope (but I welcome such a separate discussion,
of course). Fortunately, no existing code misuses parsedSize() today AFAIK.


> as soon as that's improved, +1.

Why?! This is not something the patch breaks. Do you want me to remove
the XXX and just pretend that we have not discovered the problem?


Hope this clarifies,

Alex.



> On Thu, Jul 23, 2015 at 12:41 AM, Alex Rousskov
> <rousskov at measurement-factory.com
> <mailto:rousskov at measurement-factory.com>> wrote:
> 
>     Hello,
> 
>         The attached patch adds SBuf and Tokenizer methods to simplify
>     suffix parsing and skipping (and to make suffix/reverse APIs more
>     similar to prefix/forward ones). The new methods are used in another
>     patch (to be posted shortly) that makes the request line parser simpler
>     and more robust, but they are useful in other contexts as well IMO.
> 
>     For consistency sake, I tried to mimic the existing Tokenizer/SBuf
>     documentation, debugging, and code duplication styles.
> 
> 
>     One old problem was detected and documented during this project: The
>     Tokenizer::parsedSize() method does not account for the successfully
>     parsed and removed suffix/trailing characters in most cases. That method
>     should be revised or removed. Solving this problem is outside this
>     project scope.
> 
> 
>     This and the following patch are based on trunk r14083. I will update to
>     the current trunk if the patches are approved. Please let me know if you
>     want to review the updated versions as well.
> 
> 
>     HTH,
> 
>     Alex.
> 
>     _______________________________________________
>     squid-dev mailing list
>     squid-dev at lists.squid-cache.org <mailto:squid-dev at lists.squid-cache.org>
>     http://lists.squid-cache.org/listinfo/squid-dev
> 
> 
> 
> 
> -- 
>     Francesco