Re: [PATCH 4/4] cygwin: Use native Win32 API for stat
- From
Dmitry Potapov <dpotapov@gmail.com>
- Date
- Sep 27, 2008, 21:54 UTC
- Message-ID
- <20080927215406.GG21650@dpotapov.dyndns.org>
- In-Reply-To
- <200809272035.03833.johannes.sixt@telecom.at>
On Sat, Sep 27, 2008 at 08:35:03PM +0200, Johannes Sixt wrote:
Show 18 quoted lines
> > > +core.cygwinNativeStat:: > > This name is *really* odd, for two reasons: > > - If I read "native" in connection with Windows, I would understand Windows's > implementation as "native". Cygwin is not native - it's a bolted-on feature. > > - This name talks about the implementation, not about its effect. > > Perhaps a better name would be core.ignoreCygwinFSFeatures, and the > description would only mention that setting this to true (the default) makes > many operations much faster, but makes it impossible to use File System > Features A and B and C in the repository. "If you need one of these features, > set this to false." > > (And after writing above paragraphs I notice, that you actually really meant > Windows's "native" stat; see how confusing the name is?)
It was Shawn's suggestion. I don't care much about the name as long as it is explained in the documentation... Therefore, I accepted what Shawn said without giving it any thought. Now, when you bring this name to my attention, I believe core.useCygwinStat (in the opposite to the current core.cygwinNativeStat) would be a better name. Your name is okay too, but a bit too long for my taste and not specific enough (I suppose Cygwin does many FS related tricks). Anyway, I don't have a strong opinion here, so just whatever most people like is fine with me :)
Show 11 quoted lines
>
> > +static inline void filetime_to_timespec(const FILETIME *ft, struct
> > timespec *ts) +{
> > + long long winTime = ((long long)ft->dwHighDateTime << 32) +
> > ft->dwLowDateTime; + winTime -= 116444736000000000LL; /* Windows to Unix
> > Epoch conversion */ + ts->tv_sec = (time_t)(winTime/10000000); /*
> > 100-nanosecond interval to seconds */ + ts->tv_nsec = (long)(winTime -
> > ts->tv_sec*10000000LL) * 100; /* nanoseconds */ +}
>
> Shorter lines in this function would be appreciated (and not just because my
> MUA can't deal with them ;).I am sorry, I did not notice that the line got longer than 80 columns. I will resent the patch once the issue with the name of the option is resolved.
Dmitry