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:05:08 |
Message-ID: | 20191122220508.kbs45ueqv3ctkohf@development |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-11-22 22:10:24 | Re: Getting psql to redisplay command after \e |
Previous Message | Ranier Vilela | 2019-11-22 21:58:55 | [PATCH] Tiny optimization. |