Am 30.07.2013 22:15, schrieb Fredrik Gustafsson:
> On Tue, Jul 30, 2013 at 09:51:51PM +0200, Jens Lehmann wrote:
>> +/*
>> + * Try to remove the "submodule.<name>" section from .gitmodules where the
>> given
>> + * path is configured. Return 0 only if a .gitmodules file was found, a
>> section
>> + * with the correct path=<path> setting was found and we could remove it.
>> + */
>> +int remove_path_from_gitmodules(const char *path)
>> +{
>> + struct strbuf sect = STRBUF_INIT;
>> + struct string_list_item *path_option;
>> +
>> + if (!file_exists(".gitmodules")) /* Do nothing without .gitmodules */
>> + return -1;
>> +
>> + if (gitmodules_is_unmerged)
>> + die(_("Cannot change unmerged .gitmodules, resolve merge
>> conflicts first"));
>> +
>> + path_option = unsorted_string_list_lookup(&config_name_for_path, path);
>> + if (!path_option) {
>> + warning(_("Could not find section in .gitmodules where
>> path=%s"), path);
>> + return -1;
>> + }
>> + strbuf_addstr(§, "submodule.");
>> + strbuf_addstr(§, path_option->util);
>> + if (git_config_rename_section_in_file(".gitmodules", sect.buf, NULL) <
>> 0) {
>> + /* Maybe the user already did that, don't error out here */
>> + warning(_("Could not remove .gitmodules entry for %s"), path);
>> + return -1;
>> + }
>> + strbuf_release(§);
>> + return 0;
>> +}
>
> This question applies for this function and a few more functions in this
> patch that has the same characteristics.
>
> If we're in a state when we need to return non-zero, we don't do any
> cleaning (that is strbuf_release()). Since this file is in the part
> called libgit AFAIK, shouldn't we always clean after us?
Right you are, thanks for bringing that up. The last return needs a
strbuf_release(), will fix that in v4.
> Would it make sense to have different return values for different
> errors?
I don't think so. The caller only wants to know if the modification
of the .gitmodules file happened or not, so he can decide if that
needs to be staged.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
