Julian Foad <julianfoad_at_btopenworld.com> writes:
>> I was a bit dubious about changing svn_hash_gets as it is currently a
>> thin wrapper around apr_hash_get.
>
> I didn't mean we should change svn_hash_gets(), I meant to avoid
> calling it if we have no hash.
The reason for creating the empty svn_config_t was avoid having to
insert, and potentially forget, those checks. Perhaps it's easy to
catch the calls in svn.c but it's harder in all the xxx-cmd.c files.
>> I feel a bit uneasy about allowing
>> NULL to be passed everywhere that svn_hash_gets is used. Are there
>> places where we need the hash to exist? I don't know if there are any
>> such places, and if there are then they should already have an explict
>> check.
>>
>>> which could be trivially conditionalized, and call
>>> svn_cmdline__apply_config_options(). That last call is the only place
>>> where we need to actually create a hash. Any reason not to do it
>>> there?
>>
>> svn_cmdline__apply_config_options might be a better place if the
>> svn_hash_gets change is acceptable.
Except it doesn't really work with the current API.
cfg_config = svn_hash_gets(cfg_hash, SVN_CONFIG_CATEGORY_CONFIG);
SVN_ERR(svn_cmdline__apply_config_options(cfg_config, ...));
svn_cmdline__apply_config_options doesn't take in the hash, it takes in
an svn_config_t. If the function were to accept a NULL svn_config_t
that would not be enough for the client as it needs to store the change.
Even if the fuction were modified to return a new svn_config_t when
passed NULL that would not be enough as it doesn't get back into the
hash.
> My main point is that the way this hash (pointer) gets from here to
> the Subversion libraries is through the call to
> svn_client_create_context2(). That is already documented to accept
> NULL. So that seems like the best place to handle any necessary
> default-construction. That also sets the precedent for whether the
> config hash pointers passed around the system generally may or may not
> be null.
The config/context handling is ugly. Pre-1.8 the config was set after
the context was created and those modifications were expected to take
effect. With 1.8 some of the config settings are only ever examined
during the call to svn_client_create_context2(), any subsequent
modification of such settings has no effect. So the svn client attempts
to fully populate the config before creating the context.
Not really sure how best to proceed.
1/ have svn_config_get_config() return the hash of empty svn_config_t
rather than NULL, perhaps with a special error
2/ introduce svn_config_create_config_hash() and call it when
svn_config_get_config() fails
3/ something else?
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Received on 2014-04-29 14:41:27 CEST