Read the third paragraph of this doc string and tell me there isn't
something wrong here:
/** Find the value of a (@a section, @a option) pair in @a cfg, set @a
* *valuep to the value.
*
* If @a cfg is @c NULL, just sets @a *valuep to @a default_value. If
* the value does not exist, expand and return @a default_value. @a
* default_value can be NULL.
*
* The returned value will be valid at least until the next call to
* svn_config_get(), or for the lifetime of @a default_value. It is
* safest to consume the returned value immediately.
*
* This function may change @a cfg by expanding option values.
*/
void svn_config_get(svn_config_t *cfg, const char **valuep,
const char *section, const char *option,
const char *default_value);
The implementation bears out the doc string's claim. (See the code
below, with notes from me marked with "###".) But this is bogosity:
another thread could call svn_config_get() and wipe out *valuep before
the first caller ever has a chance to "consume" (e.g., copy) the value.
Do we need svn_config_get2()?
void
svn_config_get(svn_config_t *cfg, const char **valuep,
const char *section, const char *option,
const char *default_value)
{
if (cfg)
{
cfg_section_t *sec;
cfg_option_t *opt = find_option(cfg, section, option, &sec);
if (opt != NULL)
{
make_string_from_option(valuep, cfg, sec, opt, NULL);
### Okay, if you trace into make_string_from_option()
### you'll see some tricky pool dances, but ultimately
### everything works out and *valuep is safe and
### permanent -- which means the lifetime caveat given in
### svn_config_get()'s doc string is unnecessary for this
### case. Now let's look at the opt == NULL case below...
}
else
{
apr_pool_t *tmp_pool = svn_pool_create(cfg->x_pool);
const char *x_default;
expand_option_value(cfg, sec, default_value, &x_default, tmp_pool);
if (x_default)
{
svn_stringbuf_set(cfg->tmp_value, x_default);
*valuep = cfg->tmp_value->data;
### Ah, this is where the lifetime caveat comes into
### play. Even though x_default is allocated in
### tmp_pool (which is about to be destroyed), we then
### memcpy it into cfg->tmp_value and thence to *valuep.
### Thus the data is safe until the next call to
### svn_config_get(), just as documented.
}
else
*valuep = default_value;
svn_pool_destroy(tmp_pool);
}
}
else
{
*valuep = default_value;
}
}
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-26 04:40:55 CET