A very superficial review, because I don't have much time, and don't
know the surrounding code well. Sorry about that.
On Tue, Feb 1, 2011 at 3:26 PM, Ramkumar Ramachandra <artagnon@xxxxxxxxx> wrote:
> diff --git a/vcs-svn/dir_cache.c b/vcs-svn/dir_cache.c
> new file mode 100644
> index 0000000..9a608ce
> --- /dev/null
> +++ b/vcs-svn/dir_cache.c
> @@ -0,0 +1,40 @@
> +/*
> + * Licensed under a two-clause BSD-style license.
> + * See LICENSE for details.
> + */
> +
> +#include "git-compat-util.h"
> +#include "string-list.h"
> +#include "line_buffer.h"
> +#include "dump_export.h"
> +
> +static struct string_list dirents = STRING_LIST_INIT_DUP;
> +static struct string_list_item *dir = NULL;
> +
> +void dir_cache_add(const char *path, enum node_kind kind) {
Style: we put the opening bracket of functions on the next line.
> + dir = string_list_insert(&dirents, path);
> + dir->util = malloc(sizeof(enum node_kind));
> + *((enum node_kind *)(dir->util)) = kind;
Unchecked malloc; perhaps you should use xmalloc instead?
> +}
> +
> +void dir_cache_remove(const char *path) {
Same style-violation as above.
> + dir = string_list_lookup(&dirents, path);
> + if (dir)
> + *((enum node_kind *)(dir->util)) = NODE_KIND_UNKNOWN;
> +}
> +
> +enum node_kind dir_cache_lookup(const char *path) {
> + dir = string_list_lookup(&dirents, path);
> + if (dir)
> + return *((enum node_kind *)(dir->util));
> + else
> + return NODE_KIND_UNKNOWN;
> +}
> +
> +void dir_cache_init() {
> + return;
> +}
> +
> +void dir_cache_deinit() {
> + string_list_clear(&dirents, 1);
> +}
Three more.
> +static void populate_revprops(struct strbuf *props, size_t author_len,
> + const char *author, size_t log_len, const char *log,
> + size_t date_len, const char *date)
> +{
> + strbuf_reset(props);
> + strbuf_addf(props, "K 10\nsvn:author\nV %lu\n%s\n", author_len, author);
> + strbuf_addf(props, "K 7\nsvn:log\nV %lu\n%s\n", log_len, log);
> + if (date_len)
> + /* SVN doesn't like an empty svn:date value */
> + strbuf_addf(props, "K 8\nsvn:date\nV %lu\n%s\n", date_len, date);
Perhaps a scope around here will make this a bit easier to read. At
first glance it looked like a missing scope to me, due to the indented
comment...
> + if (!val) die("Malformed author line");
> + if (!(tz_off = strrchr(val, ' '))) goto error;
> + *tz_off++ = '\0';
> + if (!(t = strrchr(val, ' '))) goto error;
style: use
"if (x)
do_stuff();"
instead of
"if (x) do_stuff();"
> + *(t - 1) = '\0'; /* Ignore '>' from email */
> + t++;
> + tz_off_buf = atoi(tz_off);
> + if (tz_off_buf > 1200 || tz_off_buf < -1200) goto error;
same
> + tm_time = time_to_tm(strtoul(t, NULL, 10), tz_off_buf);
> + strftime(time_buf, SVN_DATE_LEN + 1, SVN_DATE_FORMAT, tm_time);
> + strbuf_add(date, time_buf, SVN_DATE_LEN);
> + if (!(t = strchr(val, '<'))) goto error;
same
> +int parse_filemodify_mode(char *val)
> +{
> + char *t;
> +
> + if (!(t = strchr(val, ' '))) goto error;
same
> +void svnload_read(void)
> +{
> + char *t, *val;
> + int len;
> +
> + while ((t = buffer_read_line(&input))) {
> + if ((val = strchr(t, ' ')))
> + *val++ = '\0';
> + len = (val ? val - t - 1 : strlen(t));
> +
> + switch (len) {
> + case 1:
> + if (!memcmp(t, "D", 1)) {
> + node_ctx.action = NODE_ACTION_DELETE;
> + } else if (!memcmp(t, "C", 1)) {
> + node_ctx.action = NODE_ACTION_ADD;
> + } else if (!memcmp(t, "R", 1)) {
> + node_ctx.action = NODE_ACTION_REPLACE;
> + } else if (!memcmp(t, "M", 1)) {
> + if (!val) goto error;
> + node_ctx.action = NODE_ACTION_CHANGE;
> + val += parse_filemodify_mode(val);
> + if (!(t = strchr(val, ' '))) goto error;
same
--
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