Re: [PATCH v3 5/5] rm: delete .gitmodules entry of submodules removed from the work tree

[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(&sect, "submodule.");
> +     strbuf_addstr(&sect, 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(&sect);
> +     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?

Would it make sense to have different return values for different
errors?

I do like the comments above the function, more functions (at least
non-static ones) should follow this good style IMHO.
-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html