Use POSIX <limits.h> for PATH_MAX?

[petterreinholdtsen]

Hi.

I noticed the code fail to build on Hurd and kFreeBSD, and one of the hurdles is the way PATH_MAX is defined in the code.

POSIX specify to include <limits.h> to find PATH_MAX, and it seem to be no need for the complicated section in config.c to find it. I suggest to rewrite it like this:

diff --git a/src/config.c b/src/config.c
index 6021151..ec6713c 100644
--- a/src/config.c
+++ b/src/config.c
@@ -38,13 +38,9 @@
 #include "utils.h"
 #include <stdlib.h>
 #include <string.h>
-#if defined(__linux__)
-#include <linux/limits.h>
-#elif defined(__NetBSD__)
-#include <limits.h>
-#elif defined(__APPLE__)
+#if defined(__APPLE__)
 #include <sys/syslimits.h>
-#elif defined(__CYGWIN__)
+#else
 #include <limits.h>
 #endif
 

I do not know what kind of platform __APPLE__ refer to, but the other platforms mentioned (Linux, NetBSD, Cygwin) are POSIX compabile and can use limits.h. It also seem to be the only sensible fallback if no special casing is needed.

[jweyrich]

I believe your source is not up to date - See https://github.com/merces/pev/commit/ccef80d9092d4762344d3f4f527f1986b678562d

__APPLE__ is defined on Darwin platforms (Mac OS, Mac OS X, macOS). FWIW, I believe it's defined on iOS too.

Although the issue might still be relevant after that change. We'll need to dig a bit deeper on this one.

[petterreinholdtsen]

[Jardel Weyrich]

I believe your source is not up to date - See https://github.com/merces/pev/commit/ccef80d9092d4762344d3f4f527f1986b678562d

You are right. I am looking at the latest release.

__APPLE__ is defined on Darwin platforms (Mac OS, Mac OS X, macOS). FWIW, I believe it's defined on iOS too.

Although the issue might still be relevant after that change. We'll need to dig a bit deeper on this one.

My proposal is to make POSIX the default, while the current code do not have POSIX as the default.

Happy hacking Petter Reinholdtsen

[boddumanohar]

To fix this, I would suggest simply defining a macro for PATH_MAX and define value like 4080 which is long enough for most practical situations. This way we can make portable.

[boddumanohar]

I am sorry, the above suggestion is actually not suggested because this results in dangerous bugs like stack overflow. I just a read an article about this: http://insanecoding.blogspot.in/2007/11/pathmax-simply-isnt.html

With PATH_MAX variable for different operating systems like for windows its 260, on Linux its 4096, on OpenBSD its 1024, how can we make this variable portable?

[petterreinholdtsen]

[Manohar Reddy]

With PATH_MAX variable for different operating systems like for windows its 260, on Linux its 4096, on OpenBSD its 1024, how can we make this variable portable?

Well, for pev I believe it is easier to ask how can we get rid of the use of PATH_MAX, and for the two cases where it is used, I believe replacing char b[PATH_MAX] with char b ==*b = malloc(strlen(s1) + strlen(s2) + 2) is easier.

-- Happy hacking Petter Reinholdtsen

[boddumanohar]

char b ==*b = malloc(strlen(s1) + strlen(s2) + 2)

What are s1 an s2 here?

[petterreinholdtsen]

[Manohar Reddy]

char b ==*b = malloc(strlen(s1) + strlen(s2) + 2)

What are s1 an s2 here?

Two example/dummy string names being joined together by snprintf() in the code. In src/config.c their real values are the return value from utils_get_homedir() and DEFAULT_CONFIG_PATH, in src/plugins.c their real values are path and filename.

-- Happy hacking Petter Reinholdtsen

[GoGoOtaku]

While I haven't tested it myself a Debian maintainer pushed some merges that fix Hurd and kFreeBSD #183 Now I have to admit that I know nothing about Hurd and only relatively limited things about BSD. So I put my trust into the Debian team to know what they are doing. This is a rather old issue without activity and it seems to me that the code base has changed a lot since then. So I will close this issue for now and if issues persist I would recommend a new issue to be opened. If issues persistently keep coming up we might add Hurd and kFreeBSD to github actions.

[davidpolverari]

I know this was already closed, but this is for informational purposes only: this particular issue was fixed by 4f38c09. Maybe @fredericopissarra didn't have this (#117) issue in mind when pushing those changes, but it also solved the PATH_MAX problem for building readpe on Hurd and kFreeBSD. As a matter of fact, the latest pev version that I packaged in Debian was 0.81 (the latest tarball release of pev) was released before that 4f38c09. I managed to make Debian GNU/Hurd and kFreeBSD builds work by backporting 4f38c09 to the Debian package, along with the fix in #183.

[GoGoOtaku]

Thank you for the clarification. I figured your PR didn't "fix" this as your changes did not seem to apply to this but it told me that you made it work in Debian which implies this issue was fixed. Knowing what fixed this is good tho. So again thank you.

[davidpolverari]

You're welcome!