On Tue, Nov 6, 2018 at 5:07 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Please remove Tell from line 18 in fd.h. To Küssnacht with him!
Thanks, done. But what is this arrow sticking through my Mac laptop's
screen...?
On Tue, Nov 6, 2018 at 6:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2018-Nov-04, Thomas Munro wrote:
> >> Here's a patch to add Windows support by supplying
> >> src/backend/port/win32/pread.c. Thoughts?
>
> > Hmm, so how easy is to detect that somebody runs read/write on fds where
> > pread/pwrite have occurred? I guess for data files it's easy to detect
> > since you'd quickly end up with corrupted files, but what about other
> > kinds of files? I wonder if we should be worrying about using this
> > interface somewhere other than fd.c and forgetting about the limitation.
>
> Yeah. I think the patch as presented is OK; it uses pread/pwrite only
> inside fd.c, which is a reasonably non-leaky abstraction. But there's
> definitely a hazard of somebody submitting a patch that depends on
> using pread/pwrite elsewhere, and then that maybe not working.
>
> What I suggest is that we *not* try to make this a completely transparent
> substitute. Instead, make the functions exported by src/port/ be
> "pg_pread" and "pg_pwrite", and inside fd.c we'd write something like
>
> #ifdef HAVE_PREAD
> #define pg_pread pread
> #endif
>
> and then refer to pg_pread/pg_pwrite in the body of that file. That
> way, if someone refers to pread and expects standard functionality
> from it, they'll get a failure on platforms not supporting it.
OK. But since we're using this from both fd.c and xlog.c, I put that
into src/include/port.h.
> FWIW, I tested the given patches on HPUX 10.20; they compiled cleanly
> and pass the core regression tests.
Thanks. I also tested the replacements by temporarily hacking my
configure script to look for the wrong function name:
-ac_fn_c_check_func "$LINENO" "pread" "ac_cv_func_pread"
+ac_fn_c_check_func "$LINENO" "preadx" "ac_cv_func_pread"
-ac_fn_c_check_func "$LINENO" "pwrite" "ac_cv_func_pwrite"
+ac_fn_c_check_func "$LINENO" "pwritex" "ac_cv_func_pwrite"
--
Thomas Munro
http://www.enterprisedb.com