Johannes Sixt schrieb:
> On Samstag, 21. Juni 2008, Junio C Hamano wrote:
>> Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:
>>> please pull the MinGW (Windows) port patch series from
>>>
>>> git://repo.or.cz/git/mingw/j6t.git for-junio
I've updated the branch (it's still based on v1.5.6). Please pull again.
>> Took a look. A quick impression.
>>
>> * Too many whitespace breakages in borrowed compat/regex.[ch] are very
>> distracting.
>
> Will fixup, no problem.
Done.
>> * Shouldn't my_mktime() if exported out of date.c be named a bit better?
>
> How about tm_to_time_t()?
Done. There's a new commit (for-junio~26) that does only the renaming.
>> * The ifdef block in git.c::main() introduces decl-after-stmt which we
>> tend to avoid, but it is much worse to solve it by adding another ifdef
>> block just to enclose decl of char *bslash at the beginning of the
>> function. Perhaps enclose it in an extra block?
The #ifdef block is gone.
>> * In sanitary_path_copy(), you left "break;" after /* (1) */ but now that
>> "break" is not inside a switch() anymore, so you are breaking out of
>> something else, aren't you? -- Ah, the clean-up phase will be no-op in
>> that case because src points at '\0'. Tricky but looks correct ;-)
>
> I'm pretty certain that it is an omission. I'll remove the 'break' in the next
> round. It's just unnecessarily tricky.
Done.
>> * There seem to be an unrelated general fix in upload-pack.c
>
> Yes, indeed. It's the fflush(pack_pipe) that could make a difference. I wonder
> why this ever worked without it. Notice that traverse_commit_list calls
> show_object() last, but show_object() never flushes pack_pipe. Are fdopen()ed
> pipes line-buffered or unbuffered?
I didn't change anything here because I don't know why the old code works
on *nix, and I only know that the change is *necessary* on Windows.
> To reduce #ifdef in other places I have some proposals. Please tell me which
> you like or dislike:
>
> * The #ifdef STRIP_EXTENSION can be removed with a conditional like this:
>
> static const char ext[] = STRIP_EXTENSION; // "" or ".exe"
> if (sizeof(ext) > 1) {
> ...
> }
Done.
> * The #ifdef in main() of git.c can be removed with a custom loop that checks
> for is_dir_sep():
>
> slash = cmd + strlen(cmd);
> while (slash > cmd && !is_dir_sep(*--slash))
> ;
> if (slash >= cmd) { // was: if (slash) {
> ...
Done in a similar way.
> * We could wrap getenv(), so that the getenv("TEMPDIR") in path.c does not
> need to be followed up with getenv("TMP") and getenv("TEMP"). I'll do that.
Done.
> * The #ifdef in setup.c, prefix_filename() could easily be removed by using
> the MINGW32 arm everywhere. This would penalize non-Windows, however,
> prefix_filename() is not performance critical.
NOT done.
>> * There is an interaction with dr/ceiling topic that is already in 'next'
>> that needs to be resolved before we merge this in 'next'.
Will take care of this next; I'm running out of time now.
The interdiff follows; I created it with diff -b to hide the whitespace
changes. As you can see, there are a few more editorial changes in
compat/mingw.c (in comments and error texts only).
-- Hannes
compat/mingw.c | 33 ++++++++++++++++++++++-----------
compat/mingw.h | 3 +++
date.c | 13 ++++++++-----
git-compat-util.h | 5 +++++
git.c | 23 +++++++++++------------
path.c | 7 -------
setup.c | 1 -
7 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index ee26df9..3a05fe7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -218,7 +218,6 @@ int mkstemp(char *template)
int gettimeofday(struct timeval *tv, void *tz)
{
- extern time_t my_mktime(struct tm *tm);
SYSTEMTIME st;
struct tm tm;
GetSystemTime(&st);
@@ -228,7 +227,7 @@ int gettimeofday(struct timeval *tv, void *tz)
tm.tm_hour = st.wHour;
tm.tm_min = st.wMinute;
tm.tm_sec = st.wSecond;
- tv->tv_sec = my_mktime(&tm);
+ tv->tv_sec = tm_to_time_t(&tm);
if (tv->tv_sec < 0)
return -1;
tv->tv_usec = st.wMilliseconds*1000;
@@ -367,6 +366,19 @@ char *mingw_getcwd(char *pointer, int len)
return ret;
}
+#undef getenv
+char *mingw_getenv(const char *name)
+{
+ char *result = getenv(name);
+ if (!result && !strcmp(name, "TMPDIR")) {
+ /* on Windows it is TMP and TEMP */
+ result = getenv("TMP");
+ if (!result)
+ result = getenv("TEMP");
+ }
+ return result;
+}
+
/*
* See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
* (Parsing C++ Command-Line Arguments)
@@ -895,13 +907,12 @@ static int one_shot;
static sig_handler_t timer_fn = SIG_DFL;
/* The timer works like this:
- * The thread, ticktack(), is basically a trivial routine that most of the
- * time only waits to receive the signal to terminate. The main thread
- * tells the thread to terminate by setting the timer_event to the signalled
+ * The thread, ticktack(), is a trivial routine that most of the time
+ * only waits to receive the signal to terminate. The main thread tells
+ * the thread to terminate by setting the timer_event to the signalled
* state.
- * But ticktack() does not wait indefinitely; instead, it interrupts the
- * wait state every now and then, namely exactly after timer's interval
- * length. At these opportunities it calls the signal handler.
+ * But ticktack() interrupts the wait state after the timer's interval
+ * length to call the signal handler.
*/
static __stdcall unsigned ticktack(void *dummy)
@@ -927,7 +938,7 @@ static int start_timer_thread(void)
error("cannot start timer thread");
} else
return errno = ENOMEM,
- error("cannot allocate resources timer");
+ error("cannot allocate resources for timer");
return 0;
}
@@ -962,11 +973,11 @@ int setitimer(int type, struct itimerval *in, struct
itimerval *out)
if (out != NULL)
return errno = EINVAL,
- error("setitmer param 3 != NULL not implemented");
+ error("setitimer param 3 != NULL not implemented");
if (!is_timeval_eq(&in->it_interval, &zero) &&
!is_timeval_eq(&in->it_interval, &in->it_value))
return errno = EINVAL,
- error("setitmer: it_interval must be zero or eq it_value");
+ error("setitimer: it_interval must be zero or eq it_value");
if (timer_thread)
stop_timer_thread();
diff --git a/compat/mingw.h b/compat/mingw.h
index 6965e3f..6bc049a 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -145,6 +145,9 @@ int mingw_open (const char *filename, int oflags, ...);
char *mingw_getcwd(char *pointer, int len);
#define getcwd mingw_getcwd
+char *mingw_getenv(const char *name);
+#define getenv mingw_getenv
+
struct hostent *mingw_gethostbyname(const char *host);
#define gethostbyname mingw_gethostbyname
diff --git a/compat/regex.c b/compat/regex.c
index 1d39e08..87b33e4 100644
diff --git a/compat/regex.h b/compat/regex.h
index 408dd21..6eb64f1 100644
diff --git a/date.c b/date.c
index d6f8bf6..35a5257 100644
--- a/date.c
+++ b/date.c
@@ -6,7 +6,10 @@
#include "cache.h"
-time_t my_mktime(struct tm *tm)
+/*
+ * This is like mktime, but without normalization of tm_wday and tm_yday.
+ */
+time_t tm_to_time_t(const struct tm *tm)
{
static const int mdays[] = {
0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
@@ -67,7 +70,7 @@ static int local_tzoffset(unsigned long time)
t = time;
localtime_r(&t, &tm);
- t_local = my_mktime(&tm);
+ t_local = tm_to_time_t(&tm);
if (t_local < t) {
eastwest = -1;
@@ -322,7 +325,7 @@ static int is_date(int year, int month, int day,
struct tm *now_tm, time_t now,
if (!now_tm)
return 1;
- specified = my_mktime(r);
+ specified = tm_to_time_t(r);
/* Be it commit time or author time, it does not make
* sense to specify timestamp way into the future. Make
@@ -572,7 +575,7 @@ int parse_date(const char *date, char *result, int maxlen)
}
/* mktime uses local timezone */
- then = my_mktime(&tm);
+ then = tm_to_time_t(&tm);
if (offset == -1)
offset = (then - mktime(&tm)) / 60;
@@ -611,7 +614,7 @@ void datestamp(char *buf, int bufsize)
time(&now);
- offset = my_mktime(localtime(&now)) - now;
+ offset = tm_to_time_t(localtime(&now)) - now;
offset /= 60;
date_string(now, offset, buf, bufsize);
diff --git a/git-compat-util.h b/git-compat-util.h
index 46fc2d3..51823ae 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -114,6 +114,10 @@
#define PATH_SEP ':'
#endif
+#ifndef STRIP_EXTENSION
+#define STRIP_EXTENSION ""
+#endif
+
#ifndef has_dos_drive_prefix
#define has_dos_drive_prefix(path) 0
#endif
@@ -143,6 +147,7 @@ extern void set_error_routine(void (*routine)(const
char *err, va_list params));
extern void set_warn_routine(void (*routine)(const char *warn, va_list
params));
extern int prefixcmp(const char *str, const char *prefix);
+extern time_t tm_to_time_t(const struct tm *tm);
#ifdef NO_MMAP
diff --git a/git.c b/git.c
index a4b0a5e..871b93c 100644
--- a/git.c
+++ b/git.c
@@ -369,15 +369,16 @@ static void handle_internal_command(int argc, const
char **argv)
{ "pack-refs", cmd_pack_refs, RUN_SETUP },
};
int i;
+ static const char ext[] = STRIP_EXTENSION;
-#ifdef STRIP_EXTENSION
- i = strlen(argv[0]) - strlen(STRIP_EXTENSION);
- if (i > 0 && !strcmp(argv[0] + i, STRIP_EXTENSION)) {
+ if (sizeof(ext) > 1) {
+ i = strlen(argv[0]) - strlen(ext);
+ if (i > 0 && !strcmp(argv[0] + i, ext)) {
char *argv0 = strdup(argv[0]);
argv[0] = cmd = argv0;
argv0[i] = '\0';
}
-#endif
+ }
/* Turn "git cmd --help" into "git help cmd" */
if (argc > 1 && !strcmp(argv[1], "--help")) {
@@ -395,8 +396,8 @@ static void handle_internal_command(int argc, const
char **argv)
int main(int argc, const char **argv)
{
- const char *cmd = argv[0] ? argv[0] : "git-help";
- char *slash = strrchr(cmd, '/');
+ const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
+ char *slash = (char *)cmd + strlen(cmd);
const char *cmd_path = NULL;
int done_alias = 0;
@@ -405,12 +406,10 @@ int main(int argc, const char **argv)
* name, and the dirname as the default exec_path
* if we don't have anything better.
*/
-#ifdef __MINGW32__
- char *bslash = strrchr(cmd, '\\');
- if (!slash || (bslash && bslash > slash))
- slash = bslash;
-#endif
- if (slash) {
+ do
+ --slash;
+ while (cmd <= slash && !is_dir_sep(*slash));
+ if (cmd <= slash) {
*slash++ = 0;
cmd_path = cmd;
cmd = slash;
diff --git a/path.c b/path.c
index 5da41c7..7a35a26 100644
--- a/path.c
+++ b/path.c
@@ -75,13 +75,6 @@ int git_mkstemp(char *path, size_t len, const char
*template)
size_t n;
tmp = getenv("TMPDIR");
-#ifdef __MINGW32__
- /* on Windows it is TMP and TEMP */
- if (!tmp)
- tmp = getenv("TMP");
- if (!tmp)
- tmp = getenv("TEMP");
-#endif
if (!tmp)
tmp = "/tmp";
n = snprintf(path, len, "%s/%s", tmp, template);
diff --git a/setup.c b/setup.c
index ec33147..8bb7b10 100644
--- a/setup.c
+++ b/setup.c
@@ -35,7 +35,6 @@ static int sanitary_path_copy(char *dst, const char *src)
if (!src[1]) {
/* (1) */
src++;
- break;
} else if (is_dir_sep(src[1])) {
/* (2) */
src += 2;
--
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
