Re: gzclose don't set properly the errno in pg_restore

Hello,

At Sun, 6 Mar 2016 22:09:20 -0300, Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com> wrote in <CAFcNs+os5ExGvXMBrBBzzuJJamoHt5-zdJdxX39nkVG0KUxwsw(at)mail(dot)gmail(dot)com>
> Hi all,
>
> I'm facing with a strange error message during a failed pg_restore:
>
> pg_restore: processing data for table "historicopesquisaitem"
> pg_restore: [directory archiver] could not close data file: Success
>
> I got this error trying to restore from a directory dump file.
>
> Looking at the pg_restore code seems that "gzclose" didn't set the 'errno'
> because I got a Z_BUF_ERROR [1] from "cfclose" return and the "errno" still
> 0 (zero).
>
> 412 if (cfclose(cfp) !=0)
> 413 exit_horribly(modulename, "could not close data file: %s\n",
> 414 strerror(errno));
>
> Should we set the errno using the gzclose return code or just add some
> check to "strerror" the "errno" or the "gzclose return code" if they are
> different?
>
> Thoughts?
>
>
> [1] http://www.zlib.net/manual.html

The problem of adding zlib error messages next to ordinary
strerror messages is finding safe errnos for the use. Are
negative numbers safe? Numbers large enough such as 1000 are
safe?

Checking if a zlib function returned error or not cannot be
checked independently from the cause function, say, in a wrapper
of strerror.

The immediate cause of the problem is that GZCLOSE is assumed to
have the interface of fclose(). It should be assumed to have the
interface of gzclose() instead even if it is actually
fclose(). The same thing can be said for GZWRITE, GZREAD and
GZEOF.

For example, adding new function pg_gzclose instead of the macro
GZCLOSE would do.

int
pg_gzclose(void *fh)
{
#ifdef HAVE_LIBZ
return gzclose(fh);
#else
int ret = fclose(fh);

return ret ? Z_ERRNO : Z_OK;
#endif
}

The Z_* macros should be defined when #ifndef HAVE_LIBZ. The
caller will be like the following.

if ((result = pg_gzclose(...)) == Z_ERRNO)
exit_horribly(..., strerror(errno));
else if (result == Z_STRAM_ERROR)
exit_horribly("...

That said, completely doing this seems a bit bothersome..

Thoughts? Opinions?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center