From: Thomas Rast <trast@inf.ethz.ch>
Date: Fri, 03 Aug 2012 08:31:17 GMT
Subject: Re: [PATCH 11/16] Read cache-tree in index-v5
Message-ID: <87haskbcmi.fsf@thomas.inf.ethz.ch>
In-Reply-To: <1343905326-23790-12-git-send-email-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

> Since the cache-tree data is saved as part of the directory data,
> we have already read it, when we want to read the cache-tree. The
> cache-tree then only has to be converted from the directory data.

I think the first sentence is wrong.  You have already read it at the
very beginning of reading the index format, when you parsed the
directory records, haven't you?

> The cache-tree isn't lexically sorted, but after the pathlen at
> each level, therefore the directories have to be reordered with
> respect to the ondisk layout.

I'm not a native speaker either, but I think this does't parse well.
Maybe

  The cache-tree data is arranged in a tree, with the children sorted by
  pathlen at each node.  So we have to rebuild this format from the
  on-disk directory list.

> +	for (i = 0; i < subtree_nr; i++) {
> +		struct cache_tree *sub;
> +		struct cache_tree_sub *subtree;
> +		char *buf, *name;
> +
> +		name = "";
> +		buf = strtok(down[i].de->pathname, "/");

man 3 strtok says

   Be cautious when using these functions.  If you do use them, note
   that:

   * These functions modify their first argument.

   * These functions cannot be used on constant strings.

   * The identity of the delimiting character is lost.

   * The strtok() function uses a static buffer while parsing, so it's
     not thread safe.  Use strtok_r() if this matters to you.

I don't think the last point will be a problem, but what about modifying
the argument?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch