Re: Strange coding in mvdistinct.c

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Strange coding in mvdistinct.c
Date: 2019-04-21 18:34:08
Message-ID: 20190421183408.lj7r4yr5icqrpu5r@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Apr 15, 2019 at 06:12:24PM -0400, Tom Lane wrote:
>Oh, and as I continue to grep, I found this in dependencies.c:
> dependencies = (MVDependencies *) repalloc(dependencies,
> offsetof(MVDependencies, deps)
> + dependencies->ndeps * sizeof(MVDependency));
>I'm pretty sure this is an actual bug: the calculation should be
> offsetof(MVDependencies, deps)
> + dependencies->ndeps * sizeof(MVDependency *));
>because deps is an array of MVDependency* not MVDependency.
>This would lead to an overallocation not underallocation, and it's
>probably pretty harmless because ndeps can't get too large (I hope;
>if it could, this would have O(N^2) performance problems). Still,
>you oughta fix it.
>(There's a similar calculation later in the file that gets it right.)

I've pushed a fix correcting those issues - both for mvndistinct and
functional dependencies. I've reworked the macros used to compute the
serialized sizes not to use offsetof(), which however made them pretty
useless for other purposes. So I've made them private by moving them to
the .c files where they are moved them to the .c files.

I don't think we need to backpatch this - we're not going to add new
fields in backbranches, so there's little danger of introducing padding.
And I'm not sure we want to remove the macros, although it's unlikely
anyone else uses them.


Tomas Vondra
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2019-04-21 18:42:18 Re: [PATCH v1] Add \echo_stderr to psql
Previous Message David Fetter 2019-04-21 18:31:15 [PATCH v1] Add \echo_stderr to psql