[MERGE][1.0] update C PatienceDiff to allow for non-strings

Lukáš Lalinský lalinsky at gmail.com
Wed Dec 5 15:48:15 GMT 2007


On St, 2007-12-05 at 09:28 -0600, John Arbash Meinel wrote:
> -    const char *data;
> +    const PyObject *data;

I think this should be just PyObject *, to avoid the const -> non-const
type casting below. 

> @@ -151,8 +151,8 @@
>  static inline int
>  compare_lines(struct line *a, struct line *b)
>  {
> -    return ((a->hash != b->hash) || (a->len != b->len) ||
> -            memcmp(a->data, b->data, a->len));
> +    return ((a->hash != b->hash) || (a->len != b->len)
> +            || PyObject_Compare((PyObject *)a->data, (PyObject *)b->data));
>  }

...

> +            /* we could use the 'djb2' hash here if we find it is
> better than
> +               PyObject_Hash, however it seems measurably slower to
> compute,
> +               and doesn't give particularly better results
> +             */

This comment doesn't apply now, I guess. :)

> +            line->len = PyString_GET_SIZE(item);
> +            p = PyString_AS_STRING(item);
> +            /* 'djb2' hash. This gives us a nice compromise between fast hash
> +                function and a hash with less collisions. The algorithm doesn't
> +                use the hash for actual lookups, only for building the table
> +                so a better hash function wouldn't bring us much benefit, but
> +                would make this loading code slower. */

> +            hash = 5381;
> +            for (j = 0; j < line->len; j++)
> +                hash = ((hash << 5) + hash) + *p++;
> +            line->hash = hash;

...

> +        if (line->hash == (-1)) {
> +            /* Propogate the hash exception */
> +            size = -1;
> +            goto cleanup;
> +        }

This is incorrect. The code above might generate hash -1 and the hash is
still valid. It should fail only if -1 is returned from PyObject_Hash.

Lukas





More information about the bazaar mailing list