Fixed realpath return pointer to allocation allocated by the LibC #12
Conversation
ebiggers
left a comment
ebiggers
left a comment
There was a problem hiding this comment.
Do you have a use case for setting a custom allocator? There are other cases in which it is known to not work, e.g. various allocations that occur in NTFS-3G and Windows modes. I feel it was kind of a mistake to support it. It makes sense in lower-level libraries like compression libraries, but wimlib is a bit higher level.
Would you mind squashing your commits and including your explanation in the commit message itself, not just in the pull request description, so it doesn't get lost? Thanks.
| #define REALLOC wimlib_realloc | ||
| #define CALLOC wimlib_calloc | ||
| #define STRDUP wimlib_strdup | ||
| #define REALPATH wimlib_realpath |
|
|
||
| tchar * | ||
| realpath(const tchar *path, tchar *resolved_path); | ||
| wimlib_realpath(const tchar *path, tchar *resolved_path); |
There was a problem hiding this comment.
Can you drop the resolved_path argument and just use tchar *wimlib_realpath(const tchar *path)? Support for non-null resolved_path is not required, so this would be simpler.
| return memdup(str, strlen(str) + 1); | ||
| } | ||
|
|
||
| char * |
There was a problem hiding this comment.
This needs to be under #ifndef _WIN32.
Note that this fix implies that using malloc and free is still fine -- it's just the mismatch that causes the problem. I think that calls into question the purpose of setting a custom allocator in the first place. If custom allocator is really to be supported, then no allocation using malloc should occur. |
2 participants
Wimlib supports setting its own allocation hooks. I tried writing custom allocator, but then I encountered error when it tried to read off-bounds. Found out the allocation comes from
LibC, to be specific, its a buffer allocated byrealpathwhen a parameter for output buffer is specified asNULL(which is withwimlibin all cases).So I created a wrapper called
wimlib_realpath, which in case of theNULLparameter, afterrealpathreturns reallocates and copies the buffer over with wimlib's allocator hook (memdup).Originally, I allocated a buffer manually, but
PATH_MAX, at least on my system has a size of one page which I considered too much and rather chose to reallocate.