From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Ranier Vilela <ranier_gyn(at)hotmail(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Tiny optmization. |
Date: | 2019-11-22 22:18:55 |
Message-ID: | 20191122221855.2hidaegtejondhn3@development |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Nov 22, 2019 at 10:10:29PM +0000, Ranier Vilela wrote:
>Hi,
>Redudant because he it's been dereferenced here:
>
>line 3410:
> cstate = BeginCopy(pstate, true, rel, NULL, InvalidOid, attnamelist, options);
>
There's no pstate dereference here. It just passed the value to
BeginCopy.
BTW please don't top post, reply inline. It's much easier to follow the
discussion.
>Best regards.
>Ranier Vilela
>
>________________________________________
>De: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
>Enviado: sexta-feira, 22 de novembro de 2019 22:05
>Para: Ranier Vilela
>Cc: pgsql-hackers(at)postgresql(dot)org
>Assunto: Re: [PATCH] Tiny optmization.
>
>On Fri, Nov 22, 2019 at 09:41:44PM +0000, Ranier Vilela wrote:
>>Hi,
>>Maybe it doesn't matter, but, I think it's worth discussing.
>>The expression "if(pstate)" is redundant or is possible null dereference.
>>
>
>Eh? Redundant with what? Why would it be a null dereference? It's a
>parameter passed from outside, and we're not checking it before. And
>the if condition is there exactly to prevent null dereference.
>
>It's generally a good idea to inspect existing callers of the modified
>function and try running tests before submitting a patch. In this case
>there's a BeginCopyFrom() call in contrib/file_fdw, passing NULL as the
>first parameter, and if you run `make check` for that module it falls
>flat on it's face due to a segfault.
>
>regards
>
>>
>>--- \dll\postgresql-12.0\a\backend\commands\copy.c Mon Sep 30 17:06:55 2019
>>+++ copy.c Fri Nov 22 18:33:05 2019
>>@@ -3426,8 +3426,7 @@
>> cstate->raw_buf_index = cstate->raw_buf_len = 0;
>>
>> /* Assign range table, we'll need it in CopyFrom. */
>>- if (pstate)
>>- cstate->range_table = pstate->p_rtable;
>>+ cstate->range_table = pstate->p_rtable;
>>
>> tupDesc = RelationGetDescr(cstate->rel);
>> num_phys_attrs = tupDesc->natts;
>
>
>
>--
>Tomas Vondra http://www.2ndQuadrant.com
>PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Ranier Vilela | 2019-11-22 22:24:05 | RE: [PATCH] Tiny optmization. |
Previous Message | Tom Lane | 2019-11-22 22:17:33 | Re: [PATCH] Tiny optmization. |