Re: Segfault when restoring -Fd dump on current HEAD

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Hubert Lubaczewski <depesz(at)depesz(dot)com>, pgsql-hackers mailing list <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Segfault when restoring -Fd dump on current HEAD
Date: 2019-02-27 17:21:08
Message-ID: CA+q6zcWCb9Gtyi3d6kkzY5DgL_1VRhEmkjwk1-SmxW=Gm5tJeg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Wed, Feb 27, 2019 at 1:32 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>
> > > I think it would be better to just put back the .defn = "" (etc) to the
> > > ArchiveEntry calls.
> >
> > Then we should do this not only for defn, but for owner and dropStmt too.
>
> Yeah, absolutely.

Done, please find the attached patch.

> > I can
> > update the fix patch I've sent before, if it's preferrable approach in this
> > particular situation.
>
> I'm not sure we need those changes, since we're forced to update all
> callsites anyway.

I guess we can keep the part about removing null checks before using strlen,
since it's going to be useless.

> On Wed, Feb 27, 2019 at 10:36 AM Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>
> > On Tue, Feb 26, 2019 at 11:53 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> >
> > On 2019-Feb-26, Dmitry Dolgov wrote:
> >
> > > Yes, it should be rather simple, we can e.g. return to the old less consistent
> > > NULL handling approach something (like in the attached patch), or replace a NULL
> > > value with an empty string in WriteToc. Give me a moment, I'll check it out. At
> > > the same time I would suggest to keep replace_line_endings -> sanitize_line,
> > > since it doesn't break compatibility.
> >
> > Hmm, shouldn't we modify sanitize_line so that it returns strdup(hyphen)
> > when input is empty and want_hyphen, too?
>
> Yes, you're right.

I've looked closer, and looks like I was mistaken. In the only place where it
matters we anyway pass NULL after verifying noOwner:

sanitized_owner = sanitize_line(ropt->noOwner ? NULL : te->owner, true);

So I haven't change sanitize_line yet, but can update it if there is a strong
opinion about this function.

Attachment Content-Type Size
0001-ArchiveEntry-null-handling.patch application/octet-stream 12.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-02-27 17:29:31 Re: Pluggable Storage - Andres's take
Previous Message Tom Lane 2019-02-27 17:02:43 Re: Segfault when restoring -Fd dump on current HEAD