On Tue, Sep 23, 2008 at 04:37:14PM +0200, Alex Riesen wrote:
> 2008/9/23 Dmitry Potapov <dpotapov@xxxxxxxxx>:
> >
> > This fast mode works only for relative paths. It is assumed that the
> > whole repository is located inside one directory without using Cygwin
> > mount to bind external paths inside of the current tree.
>
> Why runtime conditional? Why conditional at all?
I thought that in rather unusual circumstances (such as using Cygwin
mount to connect separately directories in one tree), this fast version
may not work. So, I made it conditional. It is runtime conditional,
because most users do not build Git themselves but install a ready
Cygwin package.
> Why not fallback
> to cygwin's slow stat on absolute pathnames like you do for symlinks?
Of course, I do:
+ if (file_name[0] == '/')
+ return cygstat (file_name, buf);
Sorry, if it was not clear from my above comment.
>
> > +/*
> > + * This are startup stubs, which choose what implementation of lstat/stat
>
> why do you need two of them? Isn't one not enough?
I did not want to give people reasons to say that I broke lstat :)
You can opt for the standard Cygwin version of it if for some reason,
this new function does not work. Now, I know only one case -- it is
when you use Cygwin mount inside of Git repo. Yet, I don't know enough
about Cygwin to be sure that there is no other cases. So, I just wanted
to be extra careful and not to break anything.
>
> > +stat_fn_t cygwin_stat_fn = cygwin_stat_choice;
> > +stat_fn_t cygwin_lstat_fn = cygwin_lstat_choice;
> ...
> > +typedef int (*stat_fn_t)(const char*, struct stat*);
> > +extern stat_fn_t cygwin_stat_fn;
> > +extern stat_fn_t cygwin_lstat_fn;
>
> extern int (*cygwin_stat_fn)(const char *, struct stat *);
>
> Is shorter, easier to read and easier to understand (for a C person).
> You don't even use the type anywhere else, it is just for the declaration sake!
I use it in description of a parameter of another function:
static int do_stat(const char *file_name, struct stat *buf, stat_fn_t cygstat)
Of course, you can avoid it here too, but the declaration will become
somewhat longer:
static int do_stat(const char *file_name, struct stat *buf,
int (*cygstat)(const char *, struct stat *));
so I am not sure that removing stat_fn_t improves readability, but if
there are other people who think so, I will correct that.
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
