Define PATH_MAX

[jubalh]

According to POSIX, PATH_MAX may or may not be defined. Let's define it in case it is not set.

Fix https://github.com/emersion/mrsh/issues/143

[bugaevc]

Unfortunately, this fix is wrong. It may fix building on systems that don't define PATH_MAX, but it doesn't fix working on systems that don't limit all paths to PATH_MAX bytes — GNU/Linux and GNU/Hurd are examples of such systems.

To properly support such systems, a program should simply not assume path length is bounded.

Here's an example of the code that is broken:

https://github.com/emersion/mrsh/blob/0da902c0ee6f443fe703498e60f266af7f12537e/frontend/readline.c#L40-L45

Here, since snprintf() is used, it will truncate the path to 4095 characters, resulting in a broken path. If this piece of code was to check snprintf() return value, it could at least detect this case and perhaps output a warning to the user, or at least not attempt to open the history file using this known-broken path. That still wouldn't behave properly, though: a proper behaviour would be to open the file despite its path being longer than some limit.

Some resources about how and why not to use PATH_MAX:

• https://eklitzke.org/path-max-is-tricky
• http://insanecoding.blogspot.com/2007/11/pathmax-simply-isnt.html
• https://www.gnu.org/software/hurd/community/gsoc/project_ideas/maxpath.html

[emersion]

These systems still define PATH_MAX.

Yes, it's better not to use PATH_MAX, but this increases the complexity of the software.

[bugaevc]

These systems still define PATH_MAX.

To the best of my knowledge, GNU/Linux does, GNU/Hurd doesn't.

Yes, it's better not to use PATH_MAX, but this increases the complexity of the software.

I've always thought that it's an explicit goal of mrsh to fully support everything POSIX, as long as the target system implements all of POSIX — not using any nice shortcuts and improvements that some systems provide over POSIX, not even conditionally, but also not imposing any limitations some systems may have (such as limiting supported paths to some fixed number of bytes) onto other systems.

Am I wrong about that? If that's not a goal of mrsh, that's completely fine by me (but it would be sad that the one shell that markets itself as fully POSIX doesn't get long paths right). Otherwise, perhaps supporting long paths properly is planned as a future improvement?

[emersion]

I think it's worth fixing, but care must be taken to not end up with complex code. This can be done by using mrsh_buffer for instance (instead of e.g. length computations).

[emersion]

Opened https://github.com/emersion/mrsh/issues/167 to track this