Re: [PATCH] run hook-scripts on WIN32

Chris Foote wrote:

>This patch, from rev 4328, gets the hooks working on WIN32 by checking for a
>script
>with an .exe, .cmd or .bat extentsion.
>
>+/* Check if the HOOK program exists and is a file, using POOL for
>+ temporary allocations. Returns the hook program if found,
>+ otherwise NULL. */
>+static const char*
>+check_hook_cmd (const char *hook, apr_pool_t *pool)
>+{
>+ svn_node_kind_t kind;
>+ const char* hook_path = hook;
>+
>+#ifdef SVN_WIN32
>+ /* For WIN32 we need to check with an added extentsion(s). */
>+ static const char* const check_extns[] = {
>+ ".exe", ".cmd", ".bat", /* ### Any other extentsions? */
>+ NULL
>+ };
>+ const char* const * extn = check_extns;
>+
>+ while (*extn)
>+ {
>+ hook_path = apr_pstrcat (pool, hook, *(extn++), 0);
>+#else
>+ {
>+#endif
>+ if ((! svn_io_check_path(hook_path, &kind, pool))
>+ && (kind == svn_node_file))
>+ {
>+ return hook_path;
>+ }
>+ }
>+
>+ return NULL;
>+}
>
>

This type of conditional compilation obscures the program flow. I'd
prefer to see somethng like the following:

static const char *
check_hook_cmd (const char *hook, apr_pool_t *pool)
{
  static const char *const check_extns[] = {
#ifndef SVN_WIN32
    "",
#else
    /* For Win32, we have to append an "executable" extension. */
    ".exe", ".cmd", ".bat", /* ### Any other extentsions? */
#endif
    NULL
  };

  const char *const *extn;
  for (extn = check_extns; *extn; ++extn)
    {
      const char *const hook_path =
        (**extn ? apr_pstrcat (pool, hook, *extn, 0) : hook);

      svn_node_kind_t kind;
      if (!svn_io_check_path (hook_path, &kind, pool)
          && kind == svn_node_file)
        return hook_path;
    }

  return NULL;
}

Appart from that, your patch seems O.K. to me.

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org