Hi,
The new version is attached.
Tom Lane <tgl@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 <michael@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