Re: some patches

On 06/09/10 06:31, Christian wrote:
> Hi Amos,
>
> some feedback from patch writer (Klaus Singvogel) ;)
>
> 1) ia64 patch
> squid-beta-3.0-ia64.patch
>
> --- squid-3.0-PRE3-20050715/src/ESIExpression.cc.orig 2005-07-03
> 17:25:08.000000000 +0200
> +++ squid-3.0-PRE3-20050715/src/ESIExpression.cc 2005-08-03
> 12:17:42.723184827 +0200
> @@ -714,6 +714,7 @@
> rv.eval = NULL; /* A literal */
> rv.valuetype = ESI_EXPR_INVALID;
> rv.valuestored = ESI_LITERAL_INVALID;
> + rv.precedence = 1; /* A literal */
>
> if (('0'<= *s&& *s<= '9') || *s == '-') {
> size_t length = strspn (s, "0123456789.");
>
> info: fix for unitialized var (Random Stack Data)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>

Okay, thanks, applied.

> 2) mem_node_64bit
> squid-beta-3.0-mem_node_64bit.patch
>
> --- squid-3.0-PRE3-20051212/src/mem_node.cc.orig 2005-11-02
> 23:19:22.000000000 +0100
> +++ squid-3.0-PRE3-20051212/src/mem_node.cc 2005-12-15
> 14:42:44.000000000 +0100
> @@ -49,7 +49,7 @@
> makeMemNodeDataOffset()
> {
> mem_node *p = 0L;
> - return int(ptrdiff_t(&p->data));
> + return long(ptrdiff_t(&p->data));
> }
>
> /*
>
> info: its a 64bit fix. a "int" has not same length on any architecture
> as a 'pointer'
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>

Thanks. The patch is wrong, the function itself returns int which
down-casts anyway and is stored in data space as an int and use later as
a pointer.

This particular code is not in fact casting a pointer. Its casting a
pointer offset. The value guaranteed to be less than sizeof(mem_node)
and which should be less than 4 bits on all architectures. A full int is
used because the size may take up to 16 bits worst-case.

I think from just looking at it making the whole chain from
_mem_node_data_offset storage type into ptrdiff_t would be acceptable.
But given what the above maths actually does it should not be necessary.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.8
   Beta testers wanted for 3.2.0.2