Re: [PATCH v2 1/6] submodules: add helper functions to determine presence of submodules

[Brandon Williams <bmwill@google.com>]

On 10/31, Stefan Beller wrote:
> On Mon, Oct 31, 2016 at 3:38 PM, Brandon Williams <bmwill@google.com> wrote:
> > +int is_submodule_checked_out(const char *path)
> > +{
> > +       int ret = 0;
> > +       struct strbuf buf = STRBUF_INIT;
> > +
> > +       strbuf_addf(&buf, "%s/.git", path);
> > +       ret = file_exists(buf.buf);
> 
> I think we can be more tight here; instead of checking
> if the file or directory exists, we should be checking if
> it is a valid git directory, i.e. s/file_exists/resolve_gitdir/
> which returns a path to the actual git dir (in case of a .gitlink)
> or NULL when nothing is found that looks like a git directory or
> pointer to it.

Sounds good.

> > +
> > +       strbuf_release(&buf);
> > +       return ret;
> > +}
> > +
> >  int parse_submodule_update_strategy(const char *value,
> >                 struct submodule_update_strategy *dst)
> >  {
> > diff --git a/submodule.h b/submodule.h
> > index d9e197a..bd039ca 100644
> > --- a/submodule.h
> > +++ b/submodule.h
> > @@ -37,6 +37,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
> >                 const char *path);
> >  int submodule_config(const char *var, const char *value, void *cb);
> >  void gitmodules_config(void);
> > +extern int is_submodule_initialized(const char *path);
> > +extern int is_submodule_checked_out(const char *path);
> 
> no need to put extern for function names. (no other functions in this
> header are extern. so local consistency maybe? I'd also claim that
> all other extern functions in headers ought to be declared without
> being extern)

From looking around at other sections of the code it seems like the
extern keyword is used for functions declared in header files. What's
the style guideline for the project say about this?

-- 
Brandon Williams