emersion / mrsh

A minimal POSIX shell
MIT License
492 stars 35 forks source link

Get rid of all PATH_MAX usage #167

Closed emersion closed 3 years ago

emersion commented 4 years ago

PATH_MAX often results in an arbitrary limit on path lengths.

See https://github.com/emersion/mrsh/pull/166#issuecomment-664242325

jubalh commented 4 years ago

afaik it's still the best there is.

emersion commented 4 years ago

What do you mean?

jubalh commented 4 years ago

Sorry only now read:

This can be done by using mrsh_buffer for instance (instead of e.g. length computations).

so you want to do this dynamically instead? How would you do https://github.com/emersion/mrsh/blob/master/builtin/cd.c#L23 though?

emersion commented 4 years ago

This is a tricky one. I don't think we can do better than calling pathconf as done in this example: https://pubs.opengroup.org/onlinepubs/009695399/functions/getcwd.html#tag_03_215_06_01

bugaevc commented 4 years ago

You can do better, though it's a little bit tricky. Allocate a buffer of some size — pathconf(".", _PC_PATH_MAX) bytes, or if you want to avoid wasting memory in the common case, about 64 bytes should be fine for the first attempt, — and call getcwd(). If it returns ERANGE, allocate a larger buffer, repeat.

jubalh commented 4 years ago

I thought about that one too. But I figured that falls under the "complicated code" he mentioned he would prefer not to use. And I can understand. BTW wouldn't that approach not generally work with PATH_MAX too if we have it defined like we have now?