Remove deprecated symbol `tm_get_real_path()` from `tagmanager/tm_source_file.c`. It is replaced with `utils_get_real_path()` in `utils.c`. Not used by any known plugins. See #3019. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3024
-- Commit Summary --
* Remove deprecated symbol: tm_get_real_path
-- File Changes --
M src/tagmanager/tm_source_file.c (24) M src/tagmanager/tm_source_file.h (6) M src/utils.c (12)
-- Patch Links --
https://github.com/geany/geany/pull/3024.patch https://github.com/geany/geany/pull/3024.diff
@xiota pushed 1 commit.
aaa28df50fd3273a100d10ea4c54ae9130ca173d move code from tagmanager/tm_source_file.c to utils.c
@xiota pushed 1 commit.
69c56907e952120b55436be9f9f7988d2e7349e1 include windows.h in utils.c
@kugel- commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
I think the `pathconf` code should be the default, and `PATH_MAX` should only be considered on Windows. On Unix, `PATH_MAX` is not necessarily a constant.
Looks like the code from `realpath(3)` man page and it discusses problems with that code.
Apparently we should be doing `realpath(name, NULL)` but I'm unsure if it's supported on our relevant platforms?
@xiota commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
I found two different manpages online that may apply to Mac... [iOS](https://developer.apple.com/library/archive/documentation/System/Conceptual/...) and [Darwin](http://manpagez.com/man/3/realpath/). The Darwin page states that it will return the "address of newly allocated memory" when the second argument is NULL. The iOS page does not mention what happens.
I can change it to use NULL if you like. It looks like the windows implementation in the file already supports that usage.
@elextr commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
but I'm unsure if it's supported on our relevant platforms
As I said on #3019 its POSIX 2008, so I would change it to use NULL and see if any platforms report that they are still in the noughties :-)
@kugel- commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
Sounds like a plan. The man page says you can pass NULL since glibc v2.3, and Ubuntu 18.04 has glibc v2.31 (and a BSD manpage agress as well) so it seems to be supported well.
@xiota commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
Should the return value be tested and if NULL fallback to preallocating the buffer with max_path? Or just let it fail if a platform doesn't support this use?
@elextr commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
On every modern Linux the pathconf value is "unsuitable for allocating memory", so that will fail too if PATH_MAX is not something sane, and IIUC on some systems its not defined. So I would never use the get_path_max() value to allocate memory, its known to fail on modern systems, whereas realpath() should work on modern systems. As the system realpath() can legitimately fail for other reasons, allocating memory if it fails is not a good idea.
I don't know if the Windows hack is needed still, lets assume so until some Windows guru says it went out with DOS :)
I would write it as (pseudocode): ``` #if windows and not have_realpath gchar *utils_get_real_path(const gchar *file_name){ the windows implementation allocating PATH_MAX (IIUC it is always defined on Windows) } #else gchar *utils_get_real_path(const gchar *file_name){ return realpath(file_name, NULL); } #endif ```
We really should check errno after calling `realpath()` since there is a GNU extension that returns a partial path on EACCESS or ENOENT, but hell we almost never check it, why start now :-)
@xiota commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
For Windows, according to [MS](https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limi...), MAX_PATH = 260 (includes zero terminator). Longer paths (up to 32767) can be used if certain path prefixes (`\?`) are used. For Win10, long paths can also be enabled with a registry key. The app must have a manifest file and it applies to only some functions.
@xiota pushed 1 commit.
92b0bedb0e3c34bd37731c62721304c8a587f733 Remove get_path_max helper function
@kugel- commented on this pull request.
@@ -2414,6 +2419,48 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+ +static int get_path_max(const char *path) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + int path_max = pathconf(path, _PC_PATH_MAX); + if (path_max <= 0) + path_max = 4096; + return path_max; +#endif
Adding the prefix seems like the easiest way to use longer paths on Windows, but changing Windows behavior would be difficult for me to test.
That is probably best left to the future guy that requires longer paths on Windows. Devs tend to checkout code directly to C:\src or similar (possibly *because* of the limitations?) so nobody came up with that requirement so far, to my knowledge.
@kugel- commented on this pull request.
@@ -2414,17 +2419,39 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+#if defined(G_OS_WIN32) && !defined(HAVE_REALPATH) +/* realpath implementation for Windows found at http://bugzilla.gnome.org/show_bug.cgi?id=342926 + * this one is better than e.g. liberty's lrealpath because this one uses Win32 API and works + * with special chars within the filename */ +static char *realpath(const char *pathname, char *resolved_path) +{ + int size; + + if (resolved_path != NULL) + { + size = GetFullPathName(pathname, PATH_MAX, resolved_path, NULL);
Previously the ANSI version was called explicitly. What was your reason to change that?
@xiota commented on this pull request.
@@ -2414,17 +2419,39 @@ void utils_start_new_geany_instance(const gchar *doc_path)
}
+#if defined(G_OS_WIN32) && !defined(HAVE_REALPATH) +/* realpath implementation for Windows found at http://bugzilla.gnome.org/show_bug.cgi?id=342926 + * this one is better than e.g. liberty's lrealpath because this one uses Win32 API and works + * with special chars within the filename */ +static char *realpath(const char *pathname, char *resolved_path) +{ + int size; + + if (resolved_path != NULL) + { + size = GetFullPathName(pathname, PATH_MAX, resolved_path, NULL);
I had read that this version of the function would automatically use the A or W version depending on whether unicode is enabled. But further reading indicates that it's not so straightforward to switch between the two versions. So will revert the change.
@xiota pushed 1 commit.
1f417c173e108d729a8696e2b1e3898baf121878 Switch back to GetFullPathNameA
@kugel- requested changes on this pull request.
if (size > PATH_MAX)
+ return NULL; + else + return resolved_path; + } + else + { + size = GetFullPathNameA(pathname, 0, NULL, NULL); + resolved_path = g_new0(char, size); + GetFullPathNameA(pathname, size, resolved_path, NULL); + return resolved_path; + } +} +#endif + + /** * Get a link-dereferenced, absolute version of a file name. * * This is similar to the POSIX `realpath` function when passed a * @c NULL argument.
I think we can be clear here and say that this *is* realpath, except on Windows.
@@ -2436,7 +2463,12 @@ void utils_start_new_geany_instance(const gchar *doc_path)
GEANY_API_SYMBOL gchar *utils_get_real_path(const gchar *file_name) { - return tm_get_real_path(file_name); + gchar *path = NULL; + + if (file_name) + path = realpath(file_name, NULL);
We don't really need to implement a wrapper for `realpath()` if the only caller always passes NULL.
I suggest this ``` gchar *path = NULL;
if (file_name) { #if defined(G_OS_WIN32) && !defined(HAVE_REALPATH) gsize size = GetFullPathNameA(pathname, 0, NULL, NULL); path = g_new0(char, size); GetFullPathNameA(pathname, size, path, NULL); #else path = realpath(file_name, NULL); #endif } return path; ```
@elextr commented on this pull request.
@@ -2436,7 +2463,12 @@ void utils_start_new_geany_instance(const gchar *doc_path)
GEANY_API_SYMBOL gchar *utils_get_real_path(const gchar *file_name) { - return tm_get_real_path(file_name); + gchar *path = NULL; + + if (file_name) + path = realpath(file_name, NULL);
Agree with @kugel-, the fatally broken `realpath()` has left the building, the projects where I have noticed all now assume the [POSIX 2008](https://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html) behaviour with NULL, nobody tries to allocate themselves, so the only choice is between Windows and POSIX.
github-comments@lists.geany.org