Hi, The new version is attached.
Tom Lane <t...@sss.pgh.pa.us> wrote: > I tried to read 0001 but really couldn't make sense of the logic > at all, because it's seriously underdocumented. At minimum you > need an API spec comment for canonicalize_path_sub, explaining > what it's supposed to do and why. I have added some comments, but I'm not sure these comments are enough or easy understand. > I did notice that you dropped the separate step to collapse > adjacent separators (i.e, reduce "foo//bar" to "foo/bar"), which > seems like probably a bad idea. Add these sources back. Michael Paquier <mich...@paquier.xyz> wrote: > for example, should explain what is returned and > why. > + isabs = is_absolute_path(path); > + tmppath = strdup(path); > If possible, it would be nice to cut any need for malloc() allocations > in this code. Thank you for advice. In this version, I do not use the malloc(). > > I concur with the upthread comments that there's little chance > > we'll commit 0003 as-is; the code-to-benefit ratio is too high. > > Instead, you might consider adding test_canonicalize_path in > > src/test/regress/regress.c, and then adding a smaller number > > of regression test cases using that. > > Sounds like a good idea to me. I would move these in misc.source for > anything that require an absolute path. I'm not fully understand this. So, I do not change the test patch. Regards, Shenhao Wang
0002-v2-some-canonicalize_path-tests.patch
Description: 0002-v2-some-canonicalize_path-tests.patch
0001-v2-make-canonicalize-path-remove-all-.-in-path.patch
Description: 0001-v2-make-canonicalize-path-remove-all-.-in-path.patch
