You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kouhei Sutou <ko...@cozmixng.org> on 2008/03/28 07:03:57 UTC

[PATCH] set_up_diff_cmd_and_options() doesn't accept NULL config

Hi,

set_up_diff_cmd_and_options()'s document says:

/* Initialize DIFF_CMD_BATON.diff_cmd and DIFF_CMD_BATON.options,
 * according to OPTIONS and CONFIG.  CONFIG may be null.
 * Allocate the fields in POOL, which should be at least as long-lived
 * as the pool DIFF_CMD_BATON itself is allocated in.
 */

But it doesn't accept NULL as CONFIG.

Thanks,
--
kou

Re: [PATCH] set_up_diff_cmd_and_options() doesn't accept NULL config

Posted by Karl Fogel <kf...@red-bean.com>.
"Kouhei Sutou" <ko...@cozmixng.org> writes:
> I think CONFIG in the document means "apr_hash_t *config". And
> I noticed that if CONFIG is NULL set_up_diff_cmd_and_options()
> causes SEGV.

Fixed in r30101.  (I was confused between svn_client_ctx_t.config and
svn_config_t, thanks for catching it.)

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] set_up_diff_cmd_and_options() doesn't accept NULL config

Posted by Senthil Kumaran S <se...@collab.net>.
Hi,

Kouhei Sutou wrote:
> I think CONFIG in the document means "apr_hash_t *config". And
> I noticed that if CONFIG is NULL set_up_diff_cmd_and_options()
> causes SEGV.

Thanks for your clarification.

-- 
Senthil Kumaran S
http://www.stylesen.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] set_up_diff_cmd_and_options() doesn't accept NULL config

Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,

2008/3/28, Senthil Kumaran S <se...@collab.net>:
> Kouhei Sutou wrote:
>
> > set_up_diff_cmd_and_options()'s document says:
> >
> > /* Initialize DIFF_CMD_BATON.diff_cmd and DIFF_CMD_BATON.options,
> >  * according to OPTIONS and CONFIG.  CONFIG may be null.
> >  * Allocate the fields in POOL, which should be at least as long-lived
> >  * as the pool DIFF_CMD_BATON itself is allocated in.
> >  */
> >
> > But it doesn't accept NULL as CONFIG.
> >
>
>  Can you explain which does not accept NULL as CONFIG?

I think CONFIG in the document means "apr_hash_t *config". And
I noticed that if CONFIG is NULL set_up_diff_cmd_and_options()
causes SEGV.

/* Initialize DIFF_CMD_BATON.diff_cmd and DIFF_CMD_BATON.options,
 * according to OPTIONS and CONFIG.  CONFIG may be null.
 * Allocate the fields in POOL, which should be at least as long-lived
 * as the pool DIFF_CMD_BATON itself is allocated in.
 */
static svn_error_t *
set_up_diff_cmd_and_options(struct diff_cmd_baton *diff_cmd_baton,
                            const apr_array_header_t *options,
                            apr_hash_t *config, apr_pool_t *pool)


Thanks,
--
kou

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] set_up_diff_cmd_and_options() doesn't accept NULL config

Posted by Senthil Kumaran S <se...@collab.net>.
Kouhei Sutou wrote:
> set_up_diff_cmd_and_options()'s document says:
> 
> /* Initialize DIFF_CMD_BATON.diff_cmd and DIFF_CMD_BATON.options,
>  * according to OPTIONS and CONFIG.  CONFIG may be null.
>  * Allocate the fields in POOL, which should be at least as long-lived
>  * as the pool DIFF_CMD_BATON itself is allocated in.
>  */
> 
> But it doesn't accept NULL as CONFIG.

Can you explain which does not accept NULL as CONFIG?

> -  const char *diff_cmd;
> +  const char *diff_cmd = NULL;
>    
> -  /* See if there is a command. */
> -  svn_config_t *cfg = apr_hash_get(config, SVN_CONFIG_CATEGORY_CONFIG,
> -                                   APR_HASH_KEY_STRING);
> -  svn_config_get(cfg, &diff_cmd, SVN_CONFIG_SECTION_HELPERS,
> -                 SVN_CONFIG_OPTION_DIFF_CMD, NULL);
> +  if (config)
> +    {
> +      /* See if there is a command. */
> +      svn_config_t *cfg = apr_hash_get(config, SVN_CONFIG_CATEGORY_CONFIG,
> +                                       APR_HASH_KEY_STRING);
> +      svn_config_get(cfg, &diff_cmd, SVN_CONFIG_SECTION_HELPERS,
> +                     SVN_CONFIG_OPTION_DIFF_CMD, NULL);
> +    }
> +
>    diff_cmd_baton->diff_cmd = diff_cmd;

Here you have set diff_cmd as NULL, as per svn_config_get doc "If @a cfg is @c 
NULL, just sets @a *valuep to @a default_value." So in this case when the if 
condition is not executed, then diff_cmd stays as NULL and there is no default 
value set as per svn_config_get's doc.

Am I missing something here?

-- 
Senthil Kumaran S
http://www.stylesen.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org