Skip to content
This repository was archived by the owner on Nov 9, 2017. It is now read-only.

[issue52] adding support for long path names on Windows #85

Closed
wants to merge 2 commits into from
Closed

[issue52] adding support for long path names on Windows #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3e1dc6
[issue52] adding support for long path names on Windows
nitram509 Aug 6, 2013
44c43e2
[issue52] using wcscpy and wcscmp now, instead of single assignments …
nitram509 Aug 7, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 41 additions & 16 deletions compat/mingw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,18 @@
#include "../run-command.h"
#include "../cache.h"

#undef MAX_PATH
#define MAX_PATH PATH_MAX

static const int delay[] = { 0, 1, 10, 20, 40 };
unsigned int _CRT_fmode = _O_BINARY;

static wchar_t * deprefix(wchar_t *path) {
if ( path && wcsncmp(path, L"\\\\?\\", 4) == 0 )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code style disagrees seriously with the code style all around in git.git...

return path + 4;
return path;
}

int err_win_to_posix(DWORD winerr)
{
int error = ENOSYS;
Expand Down Expand Up @@ -337,11 +346,11 @@ int mingw_open (const char *filename, int oflags, ...)
mode = va_arg(args, int);
va_end(args);

if (filename && !strcmp(filename, "/dev/null"))
filename = "nul";

if (xutftowcs_path(wfilename, filename) < 0)
if (filename && !strcmp(filename, "/dev/null")) {
wcscpy(wfilename, L"nul\0");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the extra NUL? And why the change in the first place? And why add the {..} when the code style clearly asks not to add curly brackets around single-line blocks?

And why repeat virtually the exact same hunk three times? Would that not strongly suggest that this handling desires nothing more than to be refactored, most likely into the xutftowcs_path() function itself?

} else if (xutftowcs_path(wfilename, filename) < 0) {
return -1;
}
fd = _wopen(wfilename, oflags, mode);

if (fd < 0 && (oflags & O_CREAT) && errno == EACCES) {
Expand Down Expand Up @@ -414,11 +423,15 @@ FILE *mingw_fopen (const char *filename, const char *otype)
if (hide_dotfiles == HIDE_DOTFILES_TRUE &&
basename((char*)filename)[0] == '.')
hide = access(filename, F_OK);
if (filename && !strcmp(filename, "/dev/null"))
filename = "nul";
if (xutftowcs_path(wfilename, filename) < 0 ||
xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
if (filename && !strcmp(filename, "/dev/null")) {
wcscpy(wfilename, L"nul\0");
} else if (xutftowcs_path(wfilename, filename) < 0) {
return NULL;
}

if (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) {
return NULL;
}
file = _wfopen(wfilename, wotype);
if (file && hide && make_hidden(wfilename))
warning("Could not mark '%s' as hidden.", filename);
Expand All @@ -433,11 +446,15 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
if (hide_dotfiles == HIDE_DOTFILES_TRUE &&
basename((char*)filename)[0] == '.')
hide = access(filename, F_OK);
if (filename && !strcmp(filename, "/dev/null"))
filename = "nul";
if (xutftowcs_path(wfilename, filename) < 0 ||
xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
if (filename && !strcmp(filename, "/dev/null")) {
wcscpy(wfilename, L"nul\0");
} else if (xutftowcs_path(wfilename, filename) < 0) {
return NULL;
}

if (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0) {
return NULL;
}
file = _wfreopen(wfilename, wotype, stream);
if (file && hide && make_hidden(wfilename))
warning("Could not mark '%s' as hidden.", filename);
Expand Down Expand Up @@ -480,7 +497,7 @@ int mingw_chdir(const char *dirname)
wchar_t wdirname[MAX_PATH];
if (xutftowcs_path(wdirname, dirname) < 0)
return -1;
return _wchdir(wdirname);
return _wchdir(deprefix(wdirname));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What makes us so certain that _wchdir() can handle a possibly-loooong wdirname?

}

int mingw_chmod(const char *filename, int mode)
Expand Down Expand Up @@ -709,12 +726,20 @@ unsigned int sleep (unsigned int seconds)
char *mingw_mktemp(char *template)
{
wchar_t wtemplate[MAX_PATH];
char template_abspath[MAX_PATH];
if (xutftowcs_path(wtemplate, template) < 0)
return NULL;
if (!_wmktemp(wtemplate))
return NULL;
if (xwcstoutf(template, wtemplate, strlen(template) + 1) < 0)
if (xwcstoutf(template_abspath, deprefix(wtemplate), sizeof(template_abspath)) < 0)
return NULL;
/* copy only the base name name back to the template */
int idxt = strlen(template);
int idxtabs = strlen(template_abspath);
int len = strlen(basename(template));
for (; len >= 0; len--) {
template[idxt--] = template_abspath[idxtabs--];
}
return template;
}

Expand Down Expand Up @@ -782,7 +807,7 @@ char *mingw_getcwd(char *pointer, int len)
wchar_t wpointer[MAX_PATH];
if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
return NULL;
if (xwcstoutf(pointer, wpointer, len) < 0)
if (xwcstoutf(pointer, deprefix(wpointer), len) < 0)
return NULL;
for (i = 0; pointer[i]; i++)
if (pointer[i] == '\\')
Expand Down Expand Up @@ -1180,7 +1205,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
wenvblk = make_environment_block(deltaenv);

memset(&pi, 0, sizeof(pi));
ret = CreateProcessW(wcmd, wargs, NULL, NULL, TRUE, flags,
ret = CreateProcessW(deprefix(wcmd), wargs, NULL, NULL, TRUE, flags,
wenvblk, dir ? wdir : NULL, &si, &pi);

free(wenvblk);
Expand Down
27 changes: 24 additions & 3 deletions compat/mingw.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
/*
* things that are not available in header files
*/
#undef PATH_MAX
#define PATH_MAX 4096

typedef int pid_t;
typedef int uid_t;
Expand Down Expand Up @@ -405,10 +407,29 @@ static inline int xutftowcs(wchar_t *wcs, const char *utf, size_t wcslen)
*/
static inline int xutftowcs_path(wchar_t *wcs, const char *utf)
{
int result = xutftowcsn(wcs, utf, MAX_PATH, -1);
if (result < 0 && errno == ERANGE)
wchar_t buf[PATH_MAX - 4];

if (!strcmp(utf, "nul")) {
/* don't prefix reserved file name 'nul' */
memcpy(wcs, &(L"nul\0"), sizeof(buf));
return 3;
}

int result = xutftowcsn(wcs, utf, PATH_MAX - 4, -1);
if (result < 0 && errno == ERANGE) {
errno = ENAMETOOLONG;
return result;
return result;
}

result = GetFullPathNameW(wcs, PATH_MAX - 4, buf, NULL);
if (wcsncmp(buf, L"\\\\?\\", 4) == 0) {
memcpy(wcs, buf, sizeof(buf));
return result;
}

wcscpy(wcs, L"\\\\?\\");
memcpy(wcs + 4, buf, sizeof(buf));
return result + 4;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion compat/win32/dirent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ DIR *opendir(const char *name)

/* append optional '/' and wildcard '*' */
if (len && !is_dir_sep(pattern[len - 1]))
pattern[len++] = '/';
pattern[len++] = '\\';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sneaky. Very sneaky. And as a consequence something that I am not willing to merge as-is.

In the very least, this hunk needs a rationale in the commit message that I must have missed.

pattern[len++] = '*';
pattern[len] = 0;

Expand Down
3 changes: 3 additions & 0 deletions compat/win32/dirent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#ifndef DIRENT_H
#define DIRENT_H

#undef MAX_PATH
#define MAX_PATH 4096
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 4096? Why not 65536?

An explanation of this choice is required, in the least.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you add the comment into the sources?

For what i remember this magic number is MAX_PATH(Linux)
And since Git is developed as a helper tool for Linux (in its strict meaning of the kernel) development, its internals are made accounting for such paths. Thus attempts to use full path length Windows can offer would most probably screw the whole program


typedef struct DIR DIR;

#define DT_UNKNOWN 0
Expand Down