Re: [PATCH] partial fix for issue 571

[ most of this mentioned over IRC to Garrett, but he asked for it to be sent
  to the list, so others can get the info, too :-) ]

On Mon, Feb 25, 2002 at 08:17:31PM -0500, Garrett Rooney wrote:
>...
> +++ ./subversion/clients/cmdline/trace-commit.c Mon Feb 25 19:57:47 2002
>...
> +enum status
> +{
> + added,
> + deleted,
> + replaced
> +};

Those are awfully generic symbols. Usually, it is best to have some kind of
prefix, even on module-private symbols.

> +struct entry_status
> +{
> + svn_boolean_t binary;
> + enum status status;
> +};

Using a structure means more headache and allocations. Two alternatives:

1) use an integer casted to/from a void*. use bitfields in the integer.
2) use singleton pointers for your five states

Garrett said he didn't like (1), so option two uses things like this:

  static const char singleton_item_added = 0;
  static const char singleton_item_added_binary = 0;
  ...
  #define ITEM_ADDED (&singleton_item_added)
  #define ITEM_ADDED_B (&singleton_item_added_binary)
  ...
  apr_hash_set(hash, key, klen, ITEM_ADDED);
  ...
  val = apr_hash_get(hash, key, klen);
  if (val == ITEM_ADDED) ...

>...
> + if (es->status == replaced)
> + printf ("Replacing %s\n", (const char *)key);

You have "binary" status here, too. If you don't print it, then the user is
losing information (they used to get 'Adding (bin) ...' here).

>...
> + es = apr_hash_get (d->added_or_deleted, printable_name->data,
> + APR_HASH_KEY_STRING);

The "rule" here is:

* if you have the key's length already, then pass it

* if you don't have the length, then do not call strlen(), but use
  APR_HASH_KEY_STRING instead (the hash code can be more efficient by
  computing the hash value and the length at the same time)

So... in these cases, you have printable_name->len which can be passed.

>...
> + es = apr_hash_get (parent_d->added_or_deleted, child_d->path->data,
> + APR_HASH_KEY_STRING);

And child_d->path->len.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org