From: | "wangsh(dot)fnst(at)fujitsu(dot)com" <wangsh(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Julien Rouhaud <rjuju123(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "andrew(at)dunslane(dot)net" <andrew(at)dunslane(dot)net>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: drop tablespace failed when location contains .. on win32 |
Date: | 2022-01-24 11:21:12 |
Message-ID: | OS3PR01MB7159F21067BE3DCC321340B0F25E9@OS3PR01MB7159.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
The new version is attached.
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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(at)paquier(dot)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
Attachment | Content-Type | Size |
---|---|---|
0002-v2-some-canonicalize_path-tests.patch | application/octet-stream | 32.1 KB |
0001-v2-make-canonicalize-path-remove-all-.-in-path.patch | application/octet-stream | 10.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2022-01-24 11:33:11 | Re: Schema variables - new implementation for Postgres 15 |
Previous Message | Pavel Borisov | 2022-01-24 10:23:33 | Re: Make mesage at end-of-recovery less scary. |