Re: Support for pg_receivexlog --format=plain|tar

On Mon, Jan 16, 2017 at 1:46 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Mon, Jan 16, 2017 at 9:12 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Where did your research point to then? :) I just read the gzip rfc
> > (http://www.zlib.org/rfc-gzip.html) which seems to call it that at
> least?
>
> Well, OK. I was not aware of this RFC. I guessed it by looking at the
> code of gzip, that uses the CRC as well. I also found some reference
> into a blog post.
>

Haha, ok. That was my first google hit, but I guess I luckily hit a better
search keyword.

I'll add a reference to the comment about it before commit.

> >> > Finally, I think we should make the error message clearly say
> >> > "compressed
> >> > segment file" - just to make things extra clear.
> >>
> >> Sure.
> >
> > AFAICT the
> > + iscompress ? "compressed" : "",
> > part of the error handling is unnecessary, because iscompressed will
> always
> > be true in that block. All the other error messages in that codepath has
> > compressed hardcoded in them, as should this one.
>
> Fat-fingered here..
>
> >> Hm. It looks that you are right. zlib goes down to _tr_flush_bits() to
> >> flush some output, but this finishes only with put_byte(). As the fd
> >> is opaque in gzFile, it would be just better to open() the file first,
> >> and then use gzdopen to get the gzFile. Let's use as well the existing
> >> fd field to save it. gzclose() closes as well the parent fd per the
> >> documentation of zlib.
> >
> > This version throws a warning:
> > walmethods.c: In function ‘dir_open_for_write’:
> > walmethods.c:170:11: warning: ‘gzfp’ may be used uninitialized in this
> > function [-Wmaybe-uninitialized]
> > f->gzfp = gzfp;
>
> gcc and clang did not complain here, what did you use?
>

gcc (Debian 4.9.2-10) 4.9.2

> I can't see that there is any code path where this can actually happen
> > though, so we should probably just initialize it to NULL at variable
> > declaration. Or do you see a path where this could actually be
> incorrect?
>
> Not that I see. All the code paths using gzfp are under
> data_dir->compression > 0.
>
> > If you agree with those two comments, I will go ahead and push with those
> > minor fixes.
>
> No problem for me, thanks for the review!
>

Committed.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/