Ramkumar Ramachandra wrote:
> string_pool uses the macros in the memory pool library to create a
> memory pool for strings and expose an API for handling the
> strings.
This interns strings so they can be compared by address. Interesting.
The use of offsets instead of pointers here mean it is possible to
back the pool by a file, if I understand correctly.
> Taken directly from David Michael Barr's svn-dump-fast-export
> repository.
Missing From: and sign-off.
[...]
> +static int node_indentity_cmp(node_t *a, node_t *b)
> +{
> + int r = node_value_cmp(a, b);
> + return r ? r : (((uintptr_t) a) > ((uintptr_t) b))
> + - (((uintptr_t) a) < ((uintptr_t) b));
nitpick: could use fewer parentheses.
return r ? r : (((uintptr_t) a > (uintptr_t) b) - ...
Are the casts to uintptr_t necessary? C99 says, under "Relational
operators", paragraph 5:
When two pointers are compared, the result depends on the relative
locations in the address space of the objects pointed to. If two
pointers to object or incomplete types both point to the same
object, or both point one past the last element of the same array
object, they compare equal. If the objects pointed to are members of
the same aggregate object,
[etc]
which seems to suggest that no, simple expressions like (a > b) should
be okay, but then it finishes with
In all other cases, the behavior is undefined.
so I guess it is safer to keep the casts.
> diff --git a/vcs-svn/string_pool.h b/vcs-svn/string_pool.h
> new file mode 100644
> index 0000000..fb9e6b8
> --- /dev/null
> +++ b/vcs-svn/string_pool.h
> @@ -0,0 +1,11 @@
> +#ifndef STRING_POOL_H_
> +#define STRING_POOL_H_
style nitpick: should use space instead of tab
> +
> +#include <stdint.h>
> +#include <stdio.h>
Should use "../git-compat-util.h" for portability: unfortunately, some
platforms git supports still don’t have stdint.h iirc.
Except as noted above,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks for the pleasant read.
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html