[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] add GIT_FAST_STAT mode for Cygwin
From:       Dmitry Potapov <dpotapov () gmail ! com>
Date:       2008-09-23 21:11:24
Message-ID: 20080923211124.GT21650 () dpotapov ! dyndns ! org
[Download RAW message or body]

On Tue, Sep 23, 2008 at 10:41:42PM +0200, Johannes Sixt wrote:
> 
> You copied the function from compat/mingw.c. There it has the meaning "Fill in 
> struct stat using Win32 API" and nothing else. Here it has the meaning "Fill 
> in struct stat using Win32 API if you can, and using cygstat() in certain 
> exceptional cases". If you stayed with the original meaning, it would be 
> slightly easier to factor out common code.

do_stat() always fills in the structure, but it can do that fast using
Win32 API or fallback on cygstat() in exceptional cases. So, I don't
think I change its meaning much, its implementation certainly differs.

> > > You do duplicate a lot of code here. Any chances to factor out the
> > > common parts?
> >
> > I don't see much common code here. Initialization of 5 variables where
> > four of them are just constants? Perhaps, the biggest common part here
> > is conversion of dwFileAttributes to st_mode, but it is still 5 lines of
> > trivial code.
> 
> Sigh. I gave a pointer how to unify the two functions (although I missed the 
> fact that the member variables are named differently). I'd appreciate if you 
> did not make it more difficult than necessary to factor out common code.

Because the stat structure is different and handling exceptional
situation is different, I don't think we can have a single do_stat
function for Cygwin and MinGW. Yet, perhaps, it is possible to
move some code in common functions even if it is just a few lines.

The first candidate is win_attr_to_st_mode(), which converts
dwFileAttributes returned by GetFileAttributesExA to st_mode.
Another possible function is that obtains and converts Win32 error
code to errno value. These function can be placed into some common
header (for example, win32.h), which will included by both
implementations. Does it make sense?


Dmitry
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]